| Message ID | 20231130220156.726263-1-jstephan@baylibre.com |
|---|---|
| Headers | show |
| Series | Several fixes around recipetool appendsrcfile(s) and oe.recipeutils.bbappend_recipe | expand |
Hello Julien, thanks again for making devtool better. I don't think there are any particularly strong opinions about the finer points; at this point you know the code better than anyone else, and so I trust you make the right calls :) Alex On Thu, 30 Nov 2023 at 23:02, Julien Stephan <jstephan@baylibre.com> wrote: > > Hi all, > > I was trying to use recipetool appendsrcfile to add a file to a recipe and I noticed several issues: > * -m is not correctly supported > * recipetool appendsrfile(s) are missing a usefull dry-run mode > * appendsrc function relies on oe.recipeutils.bbappend_recipe but > duplicates some code: it constructs itself the new src_uri entry > although oe.recipeutils.bbappend_recipe is already doing it > * we are lacking a mode to patch the recipe itself > > So this series tries to fix the issues above, fix the selftest > accordingly and add new test for -m and for "patch mode" (update recipe) > and also add a way to specify the name of the file to add > (in oe.recipeutils.bbappend_recipe) > > There is still one point I would like to discuss: > I skip test_recipetool_appendsrcfile_existing_in_src_uri_diff_params > test because recipetool appendsrcfile(s) used to not add new src_uri entry > if the entry already exist even with different parameters while > oe.recipeutils.bbappend_recipe adds it if parameters are different. So > we need to figure out if we want to keep the old behaviour and if we need > to patch oe.recipeutils.bbappend_recipe, and update the test or remove > it. > I feel like if the parameters are different then we should add the new > src_entry (so old behaviour was not correct) and remove old entry so > either use "SRC_URI:remove" in bbapend file or completely remove the old > entry in patch mode > > What do you think? > > Cheers > Julien > > Julien Stephan (7): > recipetool: appendsrcfile(s): add dry-run mode > recipeutils: bbappend_recipe: fix undefined variable > recipetool: appendsrcfile(s): use params instead of extraline > recipeutils: bbappend_recipe: fix file not added to SRC_URI if name is > specified > recipeutils: bbappend_recipe: allow to patch the recipe itself > recipetool: appendsrcfile(s): add a mode to update the recipe itself > oeqa/selftest/recipetool: appendsrfile: add test for machine > > meta/lib/oe/recipeutils.py | 51 +++++++++++------- > meta/lib/oeqa/selftest/cases/recipetool.py | 30 +++++++++-- > scripts/lib/recipetool/append.py | 60 ++++++++++++++++------ > 3 files changed, 103 insertions(+), 38 deletions(-) > > -- > 2.42.0 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#191544): https://lists.openembedded.org/g/openembedded-core/message/191544 > Mute This Topic: https://lists.openembedded.org/mt/102903929/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Thu, 2023-11-30 at 23:01 +0100, Julien Stephan wrote: > Hi all, > > I was trying to use recipetool appendsrcfile to add a file to a recipe and I noticed several issues: > * -m is not correctly supported > * recipetool appendsrfile(s) are missing a usefull dry-run mode > * appendsrc function relies on oe.recipeutils.bbappend_recipe but > duplicates some code: it constructs itself the new src_uri entry > although oe.recipeutils.bbappend_recipe is already doing it > * we are lacking a mode to patch the recipe itself > > So this series tries to fix the issues above, fix the selftest > accordingly and add new test for -m and for "patch mode" (update recipe) > and also add a way to specify the name of the file to add > (in oe.recipeutils.bbappend_recipe) > > There is still one point I would like to discuss: > I skip test_recipetool_appendsrcfile_existing_in_src_uri_diff_params > test because recipetool appendsrcfile(s) used to not add new src_uri entry > if the entry already exist even with different parameters while > oe.recipeutils.bbappend_recipe adds it if parameters are different. So > we need to figure out if we want to keep the old behaviour and if we need > to patch oe.recipeutils.bbappend_recipe, and update the test or remove > it. > I feel like if the parameters are different then we should add the new > src_entry (so old behaviour was not correct) and remove old entry so > either use "SRC_URI:remove" in bbapend file or completely remove the old > entry in patch mode > > What do you think? I don't know the code well but I think that adding the src_uri if the parameters are different is the right thing to do. We should probably therefore update the tests accordingly. Thanks for working through these kinds of details! Cheers, Richard
Hi all, I was trying to use recipetool appendsrcfile to add a file to a recipe and I noticed several issues: * -m is not correctly supported * recipetool appendsrfile(s) are missing a usefull dry-run mode * appendsrc function relies on oe.recipeutils.bbappend_recipe but duplicates some code: it constructs itself the new src_uri entry although oe.recipeutils.bbappend_recipe is already doing it * we are lacking a mode to patch the recipe itself So this series tries to fix the issues above, fix the selftest accordingly and add new test for -m and for "patch mode" (update recipe) and also add a way to specify the name of the file to add (in oe.recipeutils.bbappend_recipe) There is still one point I would like to discuss: I skip test_recipetool_appendsrcfile_existing_in_src_uri_diff_params test because recipetool appendsrcfile(s) used to not add new src_uri entry if the entry already exist even with different parameters while oe.recipeutils.bbappend_recipe adds it if parameters are different. So we need to figure out if we want to keep the old behaviour and if we need to patch oe.recipeutils.bbappend_recipe, and update the test or remove it. I feel like if the parameters are different then we should add the new src_entry (so old behaviour was not correct) and remove old entry so either use "SRC_URI:remove" in bbapend file or completely remove the old entry in patch mode What do you think? Cheers Julien Julien Stephan (7): recipetool: appendsrcfile(s): add dry-run mode recipeutils: bbappend_recipe: fix undefined variable recipetool: appendsrcfile(s): use params instead of extraline recipeutils: bbappend_recipe: fix file not added to SRC_URI if name is specified recipeutils: bbappend_recipe: allow to patch the recipe itself recipetool: appendsrcfile(s): add a mode to update the recipe itself oeqa/selftest/recipetool: appendsrfile: add test for machine meta/lib/oe/recipeutils.py | 51 +++++++++++------- meta/lib/oeqa/selftest/cases/recipetool.py | 30 +++++++++-- scripts/lib/recipetool/append.py | 60 ++++++++++++++++------ 3 files changed, 103 insertions(+), 38 deletions(-)