| Message ID | 20240108143857.2316-4-jermain.horsman@nedap.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitbake-layers: Add possibility to update layers setup | expand |
There are a few additional remarks that I'd like some input on. > + logger.error("No repository specified. Please provide one using '--repository REPOSITORY'.") > + raise Exception("No repository specified. Please provide one using '--repository REPOSITORY'.") I've included both an error message and an exception here, the exception is what was used elsewhere already. However, this doesn't provide clear feedback on what exactly is wrong, using an error message will provide clear feedback in my opinion. (It's a very obvious red message after all, and not just a traceback that needs to be analysed.) On the other hand, only using an error message isn’t quite sufficient either, as it will not exit with an error code. > + parser.add_argument('--repository', '-p', I've named this option '--repository' as I would like to use '--update' with '--repository', without '--reference' at some point, in which case '--use-custom-reference' does not seem appropriate. I'm open to suggestions and I'd like to know what people think about this. Sincerely, Jermain Horsman
On Mon, 8 Jan 2024 at 15:42, Jermain Horsman <jermain.horsman@nedap.com> wrote: All the new options need longer, more detailed descriptions. > + parser.add_argument('--update', '-u', > + action='store_true', > + help='Updates an existing layers setup.') "Updates an existing layer setup json file with custom references provided via --repository and --reference options instead of writing a new json file." > + parser.add_argument('--repository', '-p', > + action='append', > + help='Repository to customize, this requires a reference to be specified.\nThis option can be used multiple times.') "Repository for which a custom reference provided with --reference would be used. This option...". > + parser.add_argument('--reference', '-r', > + help="Reference to use when updating repositories.\nThe value can be any git reference, however it is up to the user to provide a valid value.\nThis option is only useful when using '--update'.") "A custom reference to use for a repository (by default the currently checked out commit id would be written out). This value can be any reference that 'git checkout' would accept, and is not checked for validity." Drop the "This option is only useful when using '--update'." because it's not accurate I think? > I've named this option '--repository' as I would like to use '--update' with '--repository', without '--reference' > at some point, in which case '--use-custom-reference' does not seem appropriate. To be honest I think we should actually combine --repository and -reference into one: --use-custom-reference repo,reference That way each repo can have its own reference, and all customization can be done with a single command line invocation, which separate --repository and --reference options do not allow. And this leaves space for other options to be added later on. Is comma (,) as a separator okay? Maybe : is better? Alex
> Is comma (,) as a separator okay? Maybe : is better?
In my opinion a comma suggest a list, whereas a semicolon
suggests a relation, so I've opted to use ':'.
I did implement a new version, it is available on the mailing list,
it should include all other feedback as well.
Sincerely,
Jermain Horsman
On Tue, 16 Jan 2024 at 12:02, Jermain Horsman <jermain.horsman@nedap.com> wrote: > I did implement a new version, it is available on the mailing list, > it should include all other feedback as well. Thanks for persevering; the last version is good, so please re-submit it without the RFC. Once it lands we can think of further tweaks, but I think this would cover the majority of situations where people want custom or floating references for some of the repos. Alex
diff --git a/meta/lib/bblayers/setupwriters/oe-setup-layers.py b/meta/lib/bblayers/setupwriters/oe-setup-layers.py index bd71ca1f51..a4bd9c8c3d 100644 --- a/meta/lib/bblayers/setupwriters/oe-setup-layers.py +++ b/meta/lib/bblayers/setupwriters/oe-setup-layers.py @@ -31,16 +31,64 @@ class OeSetupLayersWriter(): with open(output, 'w') as f: json.dump(repos, f, sort_keys=True, indent=4) + def _read_repo_config(self, json_path): + with open(json_path) as f: + json_config = json.load(f) + + supported_versions = ["1.0"] + if json_config["version"] not in supported_versions: + raise Exception("File {} has version {}, which is not in supported versions: {}".format(json_path, json_config["version"], supported_versions)) + + return json_config + + def _modify_repo_config(self, json_config, args): + sources = json_config['sources'] + for repo in args.repository: + if not repo in sources.keys(): + raise Exception("Repository {} does not exist in setup-layers config".format(repo)) + + layer_remote = json_config['sources'][repo]['git-remote'] + layer_remote['rev'] = args.reference + # Clear describe + layer_remote['describe'] = '' + def do_write(self, parent, args): """ Writes out a python script and a json config that replicate the directory structure and revisions of the layers in a current build. """ - if not os.path.exists(args.destdir): - os.makedirs(args.destdir) - repos = parent.make_repo_config(args.destdir) - json = {"version":"1.0","sources":repos} - if not repos: - raise Exception("Could not determine layer sources") output = args.output_prefix or "setup-layers" - output = os.path.join(os.path.abspath(args.destdir),output) + output = os.path.join(os.path.abspath(args.destdir), output) + + if args.update: + # Modify existing layers setup + if args.repository is None: + logger.error("No repository specified. Please provide one using '--repository REPOSITORY'.") + raise Exception("No repository specified. Please provide one using '--repository REPOSITORY'.") + if args.reference is None: + logger.error("No reference specified. Please provide one using '--reference REFERENCE'.") + raise Exception("No reference specified. Please provide one using '--reference REFERENCE'.") + + json = self._read_repo_config(output + ".json") + if not 'sources' in json.keys(): + raise Exception("File {}.json does not contain valid layer sources.".format(output)) + + else: + # Create new layers setup + if args.repository is not None: + if args.reference is None: + logger.error("No reference specified. Please provide one using '--reference REFERENCE'.") + raise Exception("No reference specified. Please provide one using '--reference REFERENCE'.") + elif args.reference is not None: + logger.warning("Reference specified, but no repository, this has no effect.") + + if not os.path.exists(args.destdir): + os.makedirs(args.destdir) + repos = parent.make_repo_config(args.destdir) + json = {"version":"1.0","sources":repos} + if not repos: + raise Exception("Could not determine layer sources") + + if args.repository is not None: + self._modify_repo_config(json, args) + self._write_json(json, output + ".json") logger.info('Created {}.json'.format(output)) if not args.json_only: @@ -50,3 +98,12 @@ class OeSetupLayersWriter(): def register_arguments(self, parser): parser.add_argument('--json-only', action='store_true', help='When using the oe-setup-layers writer, write only the layer configuruation in json format. Otherwise, also a copy of scripts/oe-setup-layers (from oe-core or poky) is provided, which is a self contained python script that fetches all the needed layers and sets them to correct revisions using the data from the json.') + + parser.add_argument('--update', '-u', + action='store_true', + help='Updates an existing layers setup.') + parser.add_argument('--repository', '-p', + action='append', + help='Repository to customize, this requires a reference to be specified.\nThis option can be used multiple times.') + parser.add_argument('--reference', '-r', + help="Reference to use when updating repositories.\nThe value can be any git reference, however it is up to the user to provide a valid value.\nThis option is only useful when using '--update'.")