Allow for custom image partition schemes.

[cmacq2]

This commit provides preliminary support for custom partitioning schemes/image file types.
For discussion refer to: issue #10:
#10

This change assumes some artefacts provided by PR #28 are available:
#28
Specifically: br_image_dir() and br_image_path() functions.

This change does not address integration within brickstrap.sh itself (should be trivial, though).

[dlech]

Looks good other than as I mentioned in #28, I'm not seeing a need for creating multiple image files at one time.

[dlech]

Please explain how you see this image registry working. Also, how does the new -I option work when you have multiple image files?

I'm envisioning having a folder called image-drivers that contains individual files for each driver. The driver used by a project is specified by setting a variable in the config file of one of the components. brickstrap searches first $(br_project_dir)/image-drivers, then $(br_script_path)/image-drivers for a matching driver. This way you don't have to mess with registering any functions. The file will just be an executable script that creates the image.

[cmacq2]

Also, how does the new -I option work when you have multiple image files

Drivers are registered with a 'name' that identifies the driver as well as a file type suffix. Names are unique. The disk image file name is generated from the basename of -I as follows:

<basename>-<driver name>.<file type suffix>

So the necessary identifying information is simply appended to the basename specified with -I.

E.g., assuming -I test:

• test-default.img
• test-my_custom_driver.suffix

Please explain how you see this image registry working.

In a follow up integration PR, we introduce a commandline option (-F or something) to select which image types to generate. This will mediate between project defaults and end-user choice.

From the configuration developer perspective:

• You may split support for different image types over components (say, fs-beagle, fs-rpi, fs-kvm).
• Each component that needs support for a custom image type contains its own custom-image.sh file.
• In the custom-image.sh you do something like this:

#
# create custom image type:
# $1: disk image file to write.
#
function my_custom_img_driver_func()
{
# ... implement it here...
}

br_register_image_type "my_driver_name" my_custom_img_driver_func "img-file-suffix"

# you can specify multiple functions and register multiple drivers in one custom-image.sh.
# just make sure the names are unique.

• Then in a config file you can adjust IMAGE_TYPES environment variable to include "my_driver_name". Alternatively you can require the user select it with -F my_driver_name. IMAGE_TYPES allows the project configuration to override the default image type when no image type has been selected through -F.
• As a project developer you should document support for additional image types and mention what components (not to mention programs on the host) are required for each additional driver.

From the end-user perspective you can specify the exact set of image types to generate, provided corresponding drivers are available either from brickstrap defaults or from the component selection, using the -F option.

[dlech]

It's getting late and I'm about to call it quits, so I haven't read your previous comment yet. However, I wanted to let you know that I just pushed a branch with some image drivers so that you can see what I am hoping for in an image driver. Feel free to make comments on it as to how you see it fitting into what you are proposing here.

[dlech]

One thing I would like to do is call a validate function early in the process to catch as many configuration errors as possible before running through the full build.

[cmacq2]

It's getting late and I'm about to call it quits, so I haven't read your previous comment yet. However, I wanted to let you know that I just pushed a branch with some image drivers so that you can see what I am hoping for in an image driver. Feel free to make comments on it as to how you see it fitting into what you are proposing here.

The thing I like:

• Support for custom settings and validating them. I think this existing PR could benefit from incorporating a mechanism for that.

The aspects which I think just don't work right at the moment, and are better addressed in this PR already:

• Copy-paste code. This model encourages copy-paste code.
• This also means potentially copy-paste bugs, reinventing of wheels, doing work twice.
• In particular: your drivers all seem to want to create the same file (<basename>-bootroot.img)
• Which precludes you from generating multiple rootfs files at the same time.
• All your drivers duplicate the check for whether this file exists, and whether -f was specified.
• Inline code instead of functions. This means you have to execute the script, instead of sourcing (otherwise the case $1 in ... esac block makes no sense). This also means you can't reuse functions that depend on state, efficiently. In particular your calls to $(br_image_path) require that the BR_IMAGE_BASE_NAME and BR_DESTDIR variables are set. There's currently nothing in brickstrap code to export those from brickstrap itself... Likewise BR_FORCE is also not available. I don't really want to contemplate the code necessary to export those through e.g. the environment to the image driver.
• The same problem actually prevents your settings feature from working correctly (in the face of arbitrary variable names).
• Similarly it doesn't gel well with the existing components mechanism which is set up to source scripts rather than execute them.

[cmacq2]

I've updated my PR to provide necessary integration bits as well.

Additionally I added support for validation of custom configuration options (register additional validator functions), and I've also integrated one of your default drivers, so you can see how that should work now. See brickstrap-image-drivers.sh for the example of how I integrated the "redundant rootfs + data" image type.

[dlech]

If you are building more than one image and the first one fails and later ones succeed, won't this function still return success? It seems like if one fails we should just be calling fail and stopping brickstrap altogether.

[cmacq2]

If you are building more than one image and the first one fails and later ones succeed, won't this function still return success?

It shouldn't. brp_create_image_types should return non-zero if any of the types fail (it uses a different variable internally so it is safe). The || means that BRP_IMAGE_RETURN_CODE is set only on failure. So the end result should be that the exit code will correspond to the one from the last image to fail.

[dlech]

got it.

[cmacq2]

It seems like if one fails we should just be calling fail and stopping brickstrap altogether.

I think it is preferable to let the create-image step continue as much as it can. During development of a project configuration it may be that you accidentally break more than one image type, so it seems preferable to have a somewhat more comprehensive report (via the error messages) than just failing immediately on the first bad image.

Also, it makes it slightly easier for an end-user to work around a broken image type: even if one image build fails, another might succeed and present a viable interim alternative.

[dlech]

<basename>-<driver name>.<file type suffix>

I would really rather not have the driver name part of the file name if I am creating only one image file.

[dlech]

To keep things consistent with the images I have been publishing, the image name should just be <dest>.img where <dest> is the -d option.

[cmacq2]

I would really rather not have the driver name part of the file name if I am creating only one image file.

Sure, that can be done. I don't really see/understand why that is necessary, though. Question: would this special case behaviour also extend to not enforcing the corresponding file suffix (i.e. -I must be passed the full filename)?

To keep things consistent with the images I have been publishing, the image name should just be...

You could also execute brickstrap a bit differently: simply specify -I. If we agree to special case the single-driver case, then it should be sufficient to specify -I the way you want it (without suffix)...

[dlech]

How about the case when you want to make multiple images using the same driver with different parameters, for example IMAGE_SIZE? Rather than trying to handle such cases in brickstrap, I propose that we just advocate the following...

Brickstrap only creates one image file when you run brickstrap ... all. The driver that is used is specified in the config file in the project. The resulting file name is <dest>.<ext> where <dest> is the -d option and <ext> is determined by the image driver.

If you want to override the default settings of the driver, then use the corresponding environment variables. e.g. IMAGE_FILE_SIZE=1800M brickstrap ... all.

If you want to override the driver, use the -l option. e.g. brickstrap ... -l beaglebone all

If you want to override the basename of the image file, use the -I option. e.g. brickstrap ... -I my-custom all (results in my-custom.img)

If you want to build additional images, then use the create-image command after the all command.

brickstrap ... -I my-project-4GB all
IMAGE_FILE_SIZE=1800M brickstrap ... -I my-project-2GB create-image
IMAGE_FILE_SIZE=900M brickstrap ... -I my-project-1GB create-image
brickstrap ... -l beaglebone -I my-project-beaglebone-4GB create-image

For now, you can abuse the create-report hook if you need to automate creating more than one image.

[cmacq2]

How about the case when you want to make multiple images using the same driver with different parameters, for example IMAGE_SIZE? Rather than trying to handle such cases in brickstrap, I propose that we just advocate the following...

Uh: you can do so right now using the exact same create-image workflow you suggest already. There is no extra handling or support in brickstrap required for that, and I certainly do not propose we add it (I don't see what this would buy us, see next paragraph).

Besides: how often would you want to do that versus wanting to make different image types? I'd think that between those two cases the scales would tip in favour of different types over sizes -- mainly because you generally either do not statically fix the size of the image or you have a very particular target in mind anyway (so only one specific size).

Being able to generate multiple types of images as part of the same build command isn't all that exotic or out-there. It's kind of how you roll with tools like yocto/open-embedded (it is one of the few features of open-embedded that is actually easy to use for a novice user).

The resulting file name is . where is the -d option and is determined by the image driver.

Let's keep the inferred default file name for the image as it is in this PR. I think the alternative has a few drawbacks:

• File extensions are not necessarily unique. Even if they were, they are also cryptic in the sense that it's not at all obvious what the actual partitioning layout of the rootfs is inside. Even if that were not the case, it's still not immediately obvious what -l option was passed to build the image.
• The timestamp embedded in the file name is actually kind of useful: it permits an incremental build workflow that preserves older images and it makes it immediately obvious when an image was built even if modification timestamps are bogus (e.g. downloaded files).

---

I think my main objection is that the proposal doesn't make the brickstrap code significantly less complicated (all it really would amount to is removing two outer loops that invoke two particular functions so you can invoke those two functions with fixed arguments directly). At the same time it also does not make brickstrap any easier to use, having to fiddle with options.

So I don't see what we gain by reverting to generating only a single fixed image, albeit one selected via commandline options.

[cmacq2]

About yocto specifically: this section of its manual explains how image generation works with yocto/open-embedded.

TL;DR summary is: you set up the variable IMAGE_FSTYPES in a local build config

[dlech]

Uh: you can do so right now using the exact same create-image workflow you suggest already.

The point that I am trying to make is that if this already works, why are we inventing a new method to do the same thing? The complexity is not in the implementation, but rather in trying to explain "if you want to make more than one image, you can do it this way, but it only works in some cases otherwise, you have to do it this other way". Just saying "do it this way" is much simpler.

how often would you want to do that versus wanting to make different image types?

Always. I can't think of a single real-life use case for me personally (distributing images for EV3, RPi1/2 and BeagleBone) that could not be accomplished by using the default image driver and environment variables (that would be set in the config of the appropriate component directory). In the example drivers I gave earlier, the two cases that I would actually use are the default and the beaglebone. However, the beaglebone is identical other than the two extra dds for the bootloader, so it doesn't really make sense to make it a separate driver. I would just make it part of the default driver and put if statements around the beaglebone specific stuff so that it is ignored unless the variables are set. In real life, I wouldn't actually generate various sized images, I was just giving that as a simple example of using a variable in the context of the existing default image driver.

Although the full truth is that the answer is neither. I don't have a single real-life case where I would want to produce more than one image from a given rootfs.

Let's keep the inferred default file name for the image as it is in this PR.

That's fine as long as there is some way I can have full control over the entire basename of the file name (not including the file extension).

The timestamp embedded in the file name is actually kind of useful

By default, (i.e. the -d option is omitted), the destination directory should have the timestamp in the directory name already. So, by using <dest> as the image basename, it should have the timestamp in it already.

---

Offtopic: The reason I keep pushing back so hard about these theoretical use cases is exactly that - they are theoretical. I maintain brickstrap because it meets a real need for me (distributing images for the ev3dev.org project). If others come along with real needs and want to modify brickstrap in a way that can meet their need as well without breaking my needs, then great. However, I am quickly getting tired of spending my time thinking about what someone might want to do instead of working on something that actually meets a real need. @BrendanSimon has a real need for something other than the default image driver, so if we can make some sort of hook for him to latch on to, then great, but I expect him to do most of the work for actually implementing the driver to meet his needs. I would like to keep brickstrap as simple and minimalistic as possible. If you are interested in something more all-encompassing, maybe checkout ELBE that I heard about recently on the emdebian mailing list.

These are great ideas that you have, I just can't justify spending the time it is taking me to keep up at this rate with your innovations and really understand what you are doing. It is unfortunate, but that is the way it is.

[dlech]

Or if you want to reverse the roles where you are the maintainer and I am a contributor that is an option too. 😄

[cmacq2]

OK: that is fair enough.

Going forward, it seems that the basic functionality of the PR is agreed on but that image generation should be restricted to a single image.

By default, (i.e. the -d option is omitted), the destination directory should have the timestamp in the directory name already. So, by using as the image basename, it should have the timestamp in it already.

This is not currently the case: by default it is actually $(pwd). I think that is actually preferable in the single image case because it means you can issue multiple create-image commands without specifying 'redundant' options or risking the default getting in your way by 'changing' due to elapsed time.

I'll readjust the PR to take your feedback into account and rework the IMAGE_TYPES variable to a DEFAULT_IMAGE_TYPE (to be used as a default only).

[cmacq2]

Ok, so how does this version look which reduces the number of images being generated in a single brickstrap back down to one and ensures you have full control over the image base name (sans suffix) through -I.

By default an image name looks like this: <destdirname>-<timestamp>-<driver>.<suffix>
Using the -I <custom_name> option it becomes: <custom_name>.<suffix>

[dlech]

Are you missing ;; here? I just tried running brickstrap from this branch and got

brickstrap-image.sh: line 90: syntax error near unexpected token `)'

Works after I add ;;

[dlech]

;;

[cmacq2]

I'll update the PR to include the fixes found.

[dlech]

You may want to wait a bit. I still can't get brickstrap to run.

[cmacq2]

I still can't get brickstrap to run.

At what point does it fail? I'm currently doing a test run and it proceeds happily to download stage with options: -I foo -p /full/path/to/my/project -c . all

[dlech]

Somewhere in brp_parse_cli_options. Haven't figured it out exactly. My command line is

brickstrap.sh -p ev3dev-jessie -c base -c debian -c ev3 all -f

[dlech]

Found the problem. #35

[dlech]

Since this is a public function, I would still rather see an error message here rather than failing silently if the parameters are bad.

[cmacq2]

Agreed. Will update the PR.

[cmacq2]

br_register_image_type now emits an error message with fail if arguments are missing or empty.

[dlech]

Thanks for compromising with me.