| Message ID | 20241101193811.2068211-1-chris.laplante@agilent.com |
|---|---|
| State | New |
| Headers | show |
| Series | devtool: modify: add DEVTOOL_PREFER_BRANCH support | expand |
Will devtool finish and devtool reset do the right thing as well when this is used? Alex On Fri 1. Nov 2024 at 20.38, Chris Laplante via lists.openembedded.org <chris.laplante=agilent.com@lists.openembedded.org> wrote: > From: Chris Laplante <chris.laplante@agilent.com> > > Provide a per-recipe opt-out for devtool's default behavior of using the > 'devtool' > branch. This is primarily intended for company-internal recipes (e.g. on > private > layers) that use floating upstream SRCREV. It is assumed that such recipes > do > not care about patches, since the upstream is 100% controlled by the user. > > In those cases, the 'devtool' branch is unnecessary and just confuses > users. > > Typical usage would be something like the following: > > BRANCH = "main" > DEVTOOL_PREFER_BRANCH = "${BRANCH}" > SRCREV = "${AUTOREV}" > SRC_URI = "git:// > intranet.company.io/whatever/libsecretsauce;branch=${BRANCH};protocol=https > <http://intranet.company.io/whatever/libsecretsauce;branch=$%7BBRANCH%7D;protocol=https> > " > > This way, when users 'devtool modify' the recipe, the 'main' branch is > checked out by default. > > Signed-off-by: Chris Laplante <chris.laplante@agilent.com> > --- > scripts/lib/devtool/standard.py | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/scripts/lib/devtool/standard.py > b/scripts/lib/devtool/standard.py > index f2440ae804..6b6d542e8b 100644 > --- a/scripts/lib/devtool/standard.py > +++ b/scripts/lib/devtool/standard.py > @@ -907,6 +907,14 @@ def modify(args, config, basepath, workspace): > seen_patches.append(origpatch) > branch_patches[branch].append(origpatch) > > + if not args.no_extract and not > args.ignore_recipe_preferred_branch and (args.branch == "devtool"): > + prefer_branch = rd.getVar("DEVTOOL_PREFER_BRANCH") > + if prefer_branch: > + logger.info("Checking out recipe preferred branch > (DEVTOOL_PREFER_BRANCH) " > + "'{0}'; use > -i/--ignore-recipe-preferred-branch or -b/--branch " > + "next time to override".format(prefer_branch)) > + bb.process.run('git checkout {0}'.format(prefer_branch), > cwd=srctree) > + > # Need to grab this here in case the source is within a > subdirectory > srctreebase = srctree > srctree = get_real_srctree(srctree, rd.getVar('S'), > rd.getVar('WORKDIR')) > @@ -2307,7 +2315,9 @@ def register_commands(subparsers, context): > group = parser_modify.add_mutually_exclusive_group() > group.add_argument('--same-dir', '-s', help='Build in same directory > as source', action="store_true") > group.add_argument('--no-same-dir', help='Force build in a separate > build directory', action="store_true") > - parser_modify.add_argument('--branch', '-b', default="devtool", > help='Name for development branch to checkout (when not using > -n/--no-extract) (default "%(default)s")') > + group = parser_modify.add_mutually_exclusive_group() > + group.add_argument('--branch', '-b', default="devtool", help='Name > for development branch to checkout (when not using -n/--no-extract) > (default "%(default)s")') > + group.add_argument('--ignore-recipe-preferred-branch', '-i', help='Do > not check out the branch indicated by DEVTOOL_PREFER_BRANCH.', > action='store_true') > parser_modify.add_argument('--no-overrides', '-O', > action="store_true", help='Do not create branches for other override > configurations') > parser_modify.add_argument('--keep-temp', help='Keep temporary > directory (for debugging)', action="store_true") > parser_modify.set_defaults(func=modify, > fixed_setup=context.fixed_setup) > -- > 2.43.0 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#206630): > https://lists.openembedded.org/g/openembedded-core/message/206630 > Mute This Topic: https://lists.openembedded.org/mt/109341115/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
Hi Alex, > Will devtool finish and devtool reset do the right thing as well when this is > used? > > Alex > > On Fri 1. Nov 2024 at 20.38, Chris Laplante via > http://lists.ope/ > nembedded.org%2F&data=05%7C02%7Cchris.laplante%40agilent.com%7C8f50 > bd87a682487c18a508dcfab13c88%7Ca9c0bc098b46420693512ba12fb4a5c0%7 > C0%7C0%7C638660886342885249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C% > 7C%7C&sdata=RLYQPy6ueVmaBE1IuIX28ADzR14kx8uEz7CdM76e8BM%3D&res > erved=0 <chris.laplante=mailto:agilent.com@lists.openembedded.org> wrote: > From: Chris Laplante <mailto:chris.laplante@agilent.com> > > Provide a per-recipe opt-out for devtool's default behavior of using the > 'devtool' > branch. This is primarily intended for company-internal recipes (e.g. on private > layers) that use floating upstream SRCREV. It is assumed that such recipes do > not care about patches, since the upstream is 100% controlled by the user. devtool reset works fine. (We have been using this patch internally for the last 6 or 7 years.) I just tested 'devtool finish' and 'devtool update-recipe', and both are broken (not by this patch) for recipes with SRCREV = "${AUTOREV}" in general. So I wonder if devtool needs to be taught to handle recipes using AUTOREV. In which case, perhaps it should just automatically do the "DEVTOOL_PREFER_BRANCH" behavior... Here is a trace of the failure, for example: INFO: Updating SRCREV in recipe recipe_git.bb (dry-run) Traceback (most recent call last): File "/home/laplante/main_yocto/sources/poky/scripts/devtool", line 350, in <module> ret = main() File "/home/laplante/main_yocto/sources/poky/scripts/devtool", line 337, in main ret = args.func(args, config, basepath, workspace) File "/home/laplante/main_yocto/sources/poky/scripts/lib/devtool/standard.py", line 1995, in update_recipe updated, _, _ = _update_recipe(args.recipename, workspace, rd, args.mode, args.append, args.wildcard_version, args.no_remove, args.initial_rev, dry_run_outdir=dry_run_outdir, no_overrides=args.no_overrides, force_patch_refresh=args.force_patch_refresh) ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/laplante/main_yocto/sources/poky/scripts/lib/devtool/standard.py", line 1955, in _update_recipe updated, appendf, removed = _update_recipe_srcrev(recipename, workspace, srctree, crd, appendlayerdir, wildcard_version, no_remove, no_report_remove, dry_run_outdir) ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/laplante/main_yocto/sources/poky/scripts/lib/devtool/standard.py", line 1642, in _update_recipe_srcrev upd_p, new_p, del_p = _export_patches(srctree, rd, old_srcrev, ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^ patches_dir) ^^^^^^^^^^^^ File "/home/laplante/main_yocto/sources/poky/scripts/lib/devtool/standard.py", line 1387, in _export_patches GitApplyTree.extractPatches(srctree, start_revs, destdir, patch_pathspec) ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/laplante/main_yocto/sources/poky/meta/lib/oe/patch.py", line 523, in extractPatches out = runcmd(["sh", "-c", " ".join(shellcmd)], os.path.join(tree, name)) File "/home/laplante/main_yocto/sources/poky/meta/lib/oe/patch.py", line 49, in runcmd raise CmdError(cmd, exitstatus >> 8, "stdout: %s\nstderr: %s" % (stdout, stderr)) oe.patch.CmdError: Command Error: 'sh -c 'git format-patch --no-signature --no-numbered AUTOINC -o /tmp/oepatchczdfrg5l -- .'' exited with 0 Output: stdout: stderr: fatal: bad revision 'AUTOINC' Thanks, Chris
> Hi Alex, > > > Will devtool finish and devtool reset do the right thing as well when > > this is used? > > > > Alex > > > > On Fri 1. Nov 2024 at 20.38, Chris Laplante via > > https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .ope > > > nembedded.org%2F&data=05%7C02%7Cchris.laplante%40agilent.com%7C8f50 > > > bd87a682487c18a508dcfab13c88%7Ca9c0bc098b46420693512ba12fb4a5c0%7 > > > C0%7C0%7C638660886342885249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > > > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C% > > > 7C%7C&sdata=RLYQPy6ueVmaBE1IuIX28ADzR14kx8uEz7CdM76e8BM%3D&res > > erved=0 <chris.laplante=mailto:agilent.com@lists.openembedded.org> > wrote: > > From: Chris Laplante <mailto:chris.laplante@agilent.com> > > > > Provide a per-recipe opt-out for devtool's default behavior of using > > the 'devtool' > > branch. This is primarily intended for company-internal recipes (e.g. > > on private > > layers) that use floating upstream SRCREV. It is assumed that such > > recipes do not care about patches, since the upstream is 100% controlled by > the user. > > devtool reset works fine. (We have been using this patch internally for the last 6 > or 7 years.) > > I just tested 'devtool finish' and 'devtool update-recipe', and both are broken > (not by this patch) for recipes with SRCREV = "${AUTOREV}" in general. So I > wonder if devtool needs to be taught to handle recipes using AUTOREV. In > which case, perhaps it should just automatically do the > "DEVTOOL_PREFER_BRANCH" behavior... Thinking a little bit more about this, I think I just want 'finish' and 'update-recipe' to be mostly no-op in this case. When I'm developing a recipe under devtool that uses DEVTOOL_PREFER_BRANCH and SRCREV = "${AUTOREV}", I'm just using it for the edit-compile-test cycle using 'devtool build' and 'devtool deploy-target'. The only helpful thing I could foresee 'finish' or 'update-recipe' doing is maybe slap my hand if I have commits that I forgot to push. Otherwise, those subcommands are not useful here. Even if we take DEVTOOL_PREFER_BRANCH out of the equation, I think that's also true for a recipe that just uses SRCREV = "${AUTOREV}". Thanks, Chris
On Sat, 2 Nov 2024 at 00:15, chris.laplante@agilent.com <chris.laplante@agilent.com> wrote: > > I just tested 'devtool finish' and 'devtool update-recipe', and both are broken > > (not by this patch) for recipes with SRCREV = "${AUTOREV}" in general. So I > > wonder if devtool needs to be taught to handle recipes using AUTOREV. In > > which case, perhaps it should just automatically do the > > "DEVTOOL_PREFER_BRANCH" behavior... > > Thinking a little bit more about this, I think I just want 'finish' and 'update-recipe' to be mostly no-op in this case. When I'm developing a recipe under devtool that uses DEVTOOL_PREFER_BRANCH and SRCREV = "${AUTOREV}", I'm just using it for the edit-compile-test cycle using 'devtool build' and 'devtool deploy-target'. The only helpful thing I could foresee 'finish' or 'update-recipe' doing is maybe slap my hand if I have commits that I forgot to push. Otherwise, those subcommands are not useful here. > > Even if we take DEVTOOL_PREFER_BRANCH out of the equation, I think that's also true for a recipe that just uses SRCREV = "${AUTOREV}". I was also wondering if this new variable is really needed. Perhaps devtool should just do the logical thing automatically when it sees AUTOREV. Alex
On Sat, 2024-11-02 at 10:14 +0100, Alexander Kanavin via lists.openembedded.org wrote: > On Sat, 2 Nov 2024 at 00:15, chris.laplante@agilent.com > <chris.laplante@agilent.com> wrote: > > > > I just tested 'devtool finish' and 'devtool update-recipe', and > > > both are broken > > > (not by this patch) for recipes with SRCREV = "${AUTOREV}" in > > > general. So I > > > wonder if devtool needs to be taught to handle recipes using > > > AUTOREV. In > > > which case, perhaps it should just automatically do the > > > "DEVTOOL_PREFER_BRANCH" behavior... > > > > Thinking a little bit more about this, I think I just want 'finish' > > and 'update-recipe' to be mostly no-op in this case. When I'm > > developing a recipe under devtool that uses DEVTOOL_PREFER_BRANCH > > and SRCREV = "${AUTOREV}", I'm just using it for the edit-compile- > > test cycle using 'devtool build' and 'devtool deploy-target'. The > > only helpful thing I could foresee 'finish' or 'update-recipe' > > doing is maybe slap my hand if I have commits that I forgot to > > push. Otherwise, those subcommands are not useful here. > > > > Even if we take DEVTOOL_PREFER_BRANCH out of the equation, I think > > that's also true for a recipe that just uses SRCREV = "${AUTOREV}". > > I was also wondering if this new variable is really needed. Perhaps > devtool should just do the logical thing automatically when it sees > AUTOREV. I'd note that you can use AUTOREV and have local patches or local files so I think we do need to support that use case. We probably need two things, firstly to make AUTOREV work properly with devtool and secondly a way to disable the branch changes. That would presumably error if SRC_URI wasn't just a single SCM url. Our devtool testsuite is quite comprehensive so I'd ask that any changes do have appropriate test cases (oe-selftest -r devtool). Cheers, Richard
Hi Richard, Alex, > > > Even if we take DEVTOOL_PREFER_BRANCH out of the equation, I think > > > that's also true for a recipe that just uses SRCREV = "${AUTOREV}". > > > > I was also wondering if this new variable is really needed. Perhaps > > devtool should just do the logical thing automatically when it sees > > AUTOREV. > > I'd note that you can use AUTOREV and have local patches or local files so I > think we do need to support that use case. > > We probably need two things, firstly to make AUTOREV work properly with > devtool and secondly a way to disable the branch changes. That would > presumably error if SRC_URI wasn't just a single SCM url. > > Our devtool testsuite is quite comprehensive so I'd ask that any changes do have > appropriate test cases (oe-selftest -r devtool). Internally, my experience has been that users do not notice the 'devtool' branch has been checked out. They go to push their changes and end up creating a bizarre 'devtool' branch on the remote, which obviously doesn’t get picked up by our build. In the case of AUTOREV without any local patches, I'd argue that by default 'devtool modify' should checkout the branch from SRC_URI. We could add a flag to override that behavior (e.g. -d), or alternatively hint to the user they can pass '--branch devtool'. Would we be OK with this? If we're worried about it breaking backwards compatibility, then I would prefer to have an escape hatch (like DEVTOOL_PREFER_BRANCH) that I can use to opt-in to the new behavior. Thanks, Chris
On Mon, 4 Nov 2024 at 15:56, chris.laplante@agilent.com <chris.laplante@agilent.com> wrote: > Internally, my experience has been that users do not notice the 'devtool' branch has been checked out. They go to push their changes and end up creating a bizarre 'devtool' branch on the remote, which obviously doesn’t get picked up by our build. > > In the case of AUTOREV without any local patches, I'd argue that by default 'devtool modify' should checkout the branch from SRC_URI. We could add a flag to override that behavior (e.g. -d), or alternatively hint to the user they can pass '--branch devtool'. > > Would we be OK with this? If we're worried about it breaking backwards compatibility, then I would prefer to have an escape hatch (like DEVTOOL_PREFER_BRANCH) that I can use to opt-in to the new behavior. I'm not worried about that at all. A command line flag that would mimic the previous behavior is fine. We're pushing against new 'opt in' variables in general, if the new behaviour is an improvement on the previous behavior, then it should be the default, and people are expected to adjust. Bleeding edge sometimes bleeds. Alex
On Mon, 2024-11-04 at 14:56 +0000, chris.laplante@agilent.com wrote: > Hi Richard, Alex, > > > > > Even if we take DEVTOOL_PREFER_BRANCH out of the equation, I > > > > think > > > > that's also true for a recipe that just uses SRCREV = > > > > "${AUTOREV}". > > > > > > I was also wondering if this new variable is really needed. > > > Perhaps > > > devtool should just do the logical thing automatically when it > > > sees > > > AUTOREV. > > > > I'd note that you can use AUTOREV and have local patches or local > > files so I > > think we do need to support that use case. > > > > We probably need two things, firstly to make AUTOREV work properly > > with > > devtool and secondly a way to disable the branch changes. That > > would > > presumably error if SRC_URI wasn't just a single SCM url. > > > > Our devtool testsuite is quite comprehensive so I'd ask that any > > changes do have > > appropriate test cases (oe-selftest -r devtool). > > Internally, my experience has been that users do not notice the > 'devtool' branch has been checked out. They go to push their changes > and end up creating a bizarre 'devtool' branch on the remote, which > obviously doesn’t get picked up by our build. > > In the case of AUTOREV without any local patches, I'd argue that by > default 'devtool modify' should checkout the branch from SRC_URI. We > could add a flag to override that behavior (e.g. -d), or > alternatively hint to the user they can pass '--branch devtool'. > > Would we be OK with this? If we're worried about it breaking > backwards compatibility, then I would prefer to have an escape hatch > (like DEVTOOL_PREFER_BRANCH) that I can use to opt-in to the new > behavior. There are a few conflicting things here. We've tried to make devtool modify behave the same whether it operates on something source control based or tarball based, in both cases with and without patches or additional config/data files. That is a good thing in general but I can see how in this case it doesn't do what the user would like. How clearly is devtool saying what it is doing? Is the user being told that a "devtool" branch is being created to hold their changes? It isn't unreasonable to hold our local changes on the basis that the user wants to write them back into the recipe so the tool does really need a way to track that. I think UI people looking at this from the bigger perspective would consider the behaviour changing depending on context to be confusing and perhaps the real issue is we're not telling the user what is going on (perhaps with some detail on *why* in the docs too)? I'm not worried about breaking compatibility, I'm worried about making the experience inconsistent. A new magic config option doesn't really make me feel that much happier, it just forks our UI into two streams making things generally more confusing to users :(. As well as making devtool clearly tell the user what it is doing, could we set up the remotes in the git repos so that pushes did the expected thing instead (i.e. map devtool back to master/main/whatever)? Cheers, Richard
Hi Richard, > > Internally, my experience has been that users do not notice the > > 'devtool' branch has been checked out. They go to push their changes > > and end up creating a bizarre 'devtool' branch on the remote, which > > obviously doesn’t get picked up by our build. > > > > In the case of AUTOREV without any local patches, I'd argue that by > > default 'devtool modify' should checkout the branch from SRC_URI. We > > could add a flag to override that behavior (e.g. -d), or alternatively > > hint to the user they can pass '--branch devtool'. > > > > Would we be OK with this? If we're worried about it breaking backwards > > compatibility, then I would prefer to have an escape hatch (like > > DEVTOOL_PREFER_BRANCH) that I can use to opt-in to the new behavior. > > There are a few conflicting things here. We've tried to make devtool modify > behave the same whether it operates on something source control based or > tarball based, in both cases with and without patches or additional config/data > files. That is a good thing in general but I can see how in this case it doesn't do > what the user would like. > > How clearly is devtool saying what it is doing? Is the user being told that a > "devtool" branch is being created to hold their changes? Currently, 'devtool modify' doesn’t say anything about creating the 'devtool' branch. It just says: INFO: Source tree extracted to /wherever INFO: Recipe <recipe> now set up to build from /wherever > It isn't unreasonable to hold our local changes on the basis that the user wants > to write them back into the recipe so the tool does really need a way to track > that. > > I think UI people looking at this from the bigger perspective would consider the > behaviour changing depending on context to be confusing and perhaps the real > issue is we're not telling the user what is going on (perhaps with some detail on > *why* in the docs too)? > > I'm not worried about breaking compatibility, I'm worried about making the > experience inconsistent. A new magic config option doesn't really make me feel > that much happier, it just forks our UI into two streams making things generally > more confusing to users :(. From my POV, it kind of makes sense to have slightly different UI behaviors, since I see at least two different types of devtool users: 1. Layer maintainers (like myself) who care about creating patches for upstream software that we probably don't control. 2. Developers (most of my colleagues) who use 'devtool' as a convenience to test and develop changes to our closed-source components. These are our AUTOREV recipes. Some (most?) of these users probably don't know about devtool's patching functionality, but that's fine since they don't need it. So to me it makes sense that the UI might be a little different for these cases. That being said, I understand if we don't want to fracture the UI. I'm perfectly happy to continue modifying our fork of poky if that's the case :). > As well as making devtool clearly tell the user what it is doing, could we set up > the remotes in the git repos so that pushes did the expected thing instead (i.e. > map devtool back to master/main/whatever)? I think trying to be crafty with remotes is going to be even more confusing to users. What I could *maybe* see doing is tinkering with some git hooks (https://git-scm.com/book/ms/v2/Customizing-Git-Git-Hooks) to warn or nudge the user somehow. Even that I fear is not worth the complications. > Cheers, > > Richard
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded- > core@lists.openembedded.org> On Behalf Of Chris Laplante via > lists.openembedded.org > Sent: den 4 november 2024 20:58 > To: Richard Purdie <richard.purdie@linuxfoundation.org>; > alex.kanavin@gmail.com > Cc: openembedded-core@lists.openembedded.org > Subject: Re: [OE-core] [PATCH] devtool: modify: add DEVTOOL_PREFER_BRANCH > support > > Hi Richard, > > > > Internally, my experience has been that users do not notice the > > > 'devtool' branch has been checked out. They go to push their changes > > > and end up creating a bizarre 'devtool' branch on the remote, which > > > obviously doesn’t get picked up by our build. > > > > > > In the case of AUTOREV without any local patches, I'd argue that by > > > default 'devtool modify' should checkout the branch from SRC_URI. We > > > could add a flag to override that behavior (e.g. -d), or alternatively > > > hint to the user they can pass '--branch devtool'. > > > > > > Would we be OK with this? If we're worried about it breaking backwards > > > compatibility, then I would prefer to have an escape hatch (like > > > DEVTOOL_PREFER_BRANCH) that I can use to opt-in to the new behavior. > > > > There are a few conflicting things here. We've tried to make devtool > modify > > behave the same whether it operates on something source control based or > > tarball based, in both cases with and without patches or additional > config/data > > files. That is a good thing in general but I can see how in this case it > doesn't do > > what the user would like. > > > > How clearly is devtool saying what it is doing? Is the user being told > that a > > "devtool" branch is being created to hold their changes? > > Currently, 'devtool modify' doesn’t say anything about creating the > 'devtool' branch. It just says: > > INFO: Source tree extracted to /wherever > INFO: Recipe <recipe> now set up to build from /wherever > > > It isn't unreasonable to hold our local changes on the basis that the > user wants > > to write them back into the recipe so the tool does really need a way to > track > > that. > > > > I think UI people looking at this from the bigger perspective would > consider the > > behaviour changing depending on context to be confusing and perhaps the > real > > issue is we're not telling the user what is going on (perhaps with some > detail on > > *why* in the docs too)? > > > > I'm not worried about breaking compatibility, I'm worried about making > the > > experience inconsistent. A new magic config option doesn't really make > me feel > > that much happier, it just forks our UI into two streams making things > generally > > more confusing to users :(. > > From my POV, it kind of makes sense to have slightly different UI > behaviors, since I see at least two different types of devtool users: > > 1. Layer maintainers (like myself) who care about creating patches for > upstream software that we probably don't control. > 2. Developers (most of my colleagues) who use 'devtool' as a convenience > to test and develop changes to our closed-source components. These are our > AUTOREV recipes. Some (most?) of these users probably don't know about > devtool's patching functionality, but that's fine since they don't need > it. > > So to me it makes sense that the UI might be a little different for these > cases. > > That being said, I understand if we don't want to fracture the UI. I'm > perfectly happy to continue modifying our fork of poky if that's the case > :). > > > As well as making devtool clearly tell the user what it is doing, could > we set up > > the remotes in the git repos so that pushes did the expected thing > instead (i.e. > > map devtool back to master/main/whatever)? > > I think trying to be crafty with remotes is going to be even more > confusing to users. What I could *maybe* see doing is tinkering with some > git hooks (https://git-scm.com/book/ms/v2/Customizing-Git-Git-Hooks) to > warn or nudge the user somehow. Even that I fear is not worth the > complications. I would strongly advise against involving hooks. I just recently managed to get rid of the hook that was involved in keeping track of the patch name when devtool modify creates a commit from a patch, and I would hate to see a new usage of hooks introduced. > > Cheers, > > > > Richard //Peter
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py index f2440ae804..6b6d542e8b 100644 --- a/scripts/lib/devtool/standard.py +++ b/scripts/lib/devtool/standard.py @@ -907,6 +907,14 @@ def modify(args, config, basepath, workspace): seen_patches.append(origpatch) branch_patches[branch].append(origpatch) + if not args.no_extract and not args.ignore_recipe_preferred_branch and (args.branch == "devtool"): + prefer_branch = rd.getVar("DEVTOOL_PREFER_BRANCH") + if prefer_branch: + logger.info("Checking out recipe preferred branch (DEVTOOL_PREFER_BRANCH) " + "'{0}'; use -i/--ignore-recipe-preferred-branch or -b/--branch " + "next time to override".format(prefer_branch)) + bb.process.run('git checkout {0}'.format(prefer_branch), cwd=srctree) + # Need to grab this here in case the source is within a subdirectory srctreebase = srctree srctree = get_real_srctree(srctree, rd.getVar('S'), rd.getVar('WORKDIR')) @@ -2307,7 +2315,9 @@ def register_commands(subparsers, context): group = parser_modify.add_mutually_exclusive_group() group.add_argument('--same-dir', '-s', help='Build in same directory as source', action="store_true") group.add_argument('--no-same-dir', help='Force build in a separate build directory', action="store_true") - parser_modify.add_argument('--branch', '-b', default="devtool", help='Name for development branch to checkout (when not using -n/--no-extract) (default "%(default)s")') + group = parser_modify.add_mutually_exclusive_group() + group.add_argument('--branch', '-b', default="devtool", help='Name for development branch to checkout (when not using -n/--no-extract) (default "%(default)s")') + group.add_argument('--ignore-recipe-preferred-branch', '-i', help='Do not check out the branch indicated by DEVTOOL_PREFER_BRANCH.', action='store_true') parser_modify.add_argument('--no-overrides', '-O', action="store_true", help='Do not create branches for other override configurations') parser_modify.add_argument('--keep-temp', help='Keep temporary directory (for debugging)', action="store_true") parser_modify.set_defaults(func=modify, fixed_setup=context.fixed_setup)