Thanks for the review, comments below. On Tue, 6 Sept 2022 at 12:20, John Ogness <john.ogn...@linutronix.de> wrote:
> So if your motivation is to change from setting TEMPLATECONF and > sourcing oe-init-build-env to using an intuitive tool, then that should > be clearly stated in the commit message. It is not correct to say that > it cannot be setup without this tool because we've been doing it for > many years now. :-) The motivation for the tool is primarily in adding a way to discover and learn about available configuration templates which until now has been completely absent. In fact I'd say that is the primary purpose of the tool. Setting up builds is secondary, and if someone wants to continue using '. oe-init-build-env', I am totally fine with that - the tool merely provides a convenient shortcut for that with a separate shell session, and reasonable defaults for choosing where the build dir would be, unless specified explicitly via -b. > > After layers have been fetched and placed into their respective locations, > > one would > > surely want to proceed to the actual build, and here's how: > > > > 1. Without arguments, the tool simply walks the ../.. of its location > > (which is the parent dir > > of poky/oe-core), and prints what templates has found. If the following is > > not enough information, > > adding '-v' will also print conf-notes.txt for each of the templates: > > For me this is not an intuitive interface. Below is a list of changes > that I would suggest: > > 1. The tool should be called "oe-setup-build-env". This matches the > source file "oe-init-build-env" and makes it clear that only a build > environment is setup and nothing will be "built". Sure, I will change that. > 2. The "-c" argument is required (also in your code), so not specifying > it should be an error. Upon printing the error message, the tool could > provide the help. Something like: > > $ oe-setup-build-env > error: no template path specified with `-c' > > usage: oe-setup-build-env -c template_path [-h] [--topdir TOPDIR] [-v] [-b > build_path] [--no-shell] > > A script that discovers available build configuration templates and sets up a > build environment based on one of them > > optional arguments: > -h, --help show this help message and exit > --topdir TOPDIR Where to look for available build configuration templates > (default is ../..). > -v Print a description for each available build > configuration template. > -c template_path Use build configuration template in template_path to set > up a build (run this script without arguments or with -v to see what is > available) > -b build_path Set up a build in build_path (run this script without > arguments or with -v to see where it would be by default) > --no-shell Create a build but do not start a shell session with it. > > (Or maybe template_path could be a positional argument instead of > specified with -c? Not sure.) This is where I have to say no. As stated, the primary purpose is to find out what is available, and the tool should do something useful in that aspect when run without any arguments. Which is what it does - '-c' is not actually required :) > 3. In the help output I think it is important to always talk about > build environments instead of builds. For example: > > Use build configuration template in template_path to set up a build > environment > > Set up a build environment in build_path > > Create a build environment but do not start a shell session within it Yes, I will correct the terminology. > 4. Listing available templates is a nice feature, but this alternate > mode of operation should have its own argument. Something like > > -l List available build configuration templates. See above please. > 5. Your comments hint that this tool might be a second step for users of > the new "oe-setup-layers" tool. That tool uses a configuration file that > has an explicit list of layers that are used. But this tool recursively > searches for templates in _any_ layer under topdir. If this tool is > indeed meant as a second step to "oe-setup-layers" then perhaps it > should also support using the configuration file so that it knows which > layers it is allowed to list (or maybe even use). > > --jsondata File containing the layer data in json format > > With the json data available, the tool could also report if the layers > are dirty or are using a different commit, before setting up the build > environment. To get the layer descriptions in json accepted and merged, I had to drop all configuration-related data from it, and keep it strictly about repos checkout and placement into correct paths. Originally, it had a list of machines, distros and config templates, per layer, and the list of layers; now all of it is gone, and only the repos remain (and repo != layer, as repos can and do contain multiple layers). There was a lot of resistance against mixing the layer setup with build setup into the same json and same tools. We can probably come up with a more tightly controlled template discovery and buildenv setup if proven necessary, but this one just works when all checked out layers are in a single top folder just above poky/oe-core, so why not keep things simple, and do away with config files altogether? Please keep in mind the target audience: it is not you or me. This is intended, first, and foremost, for first-timers, and the expectation is that distro maintainers set things up for them (layer config, and build templates), so that nobody has to 'learn yocto' or read documentation (that includes READMEs in product/company layers too), or poke around the confusing layer tree to get from a newly created empty 'my-yocto-stuff' directory to having flashable target artifacts. Alex
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#170347): https://lists.openembedded.org/g/openembedded-core/message/170347 Mute This Topic: https://lists.openembedded.org/mt/93485066/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
