Minor enhancements/fixes.

[cmacq2]

• Fix: delete destination directory if possible & sensible. See issue #34 https://github.com/ev3dev/brickstrap/issues/34
• Make it easier to detect in brickstrap code if '-d' option was specified by moving the default destination directory value to its own variable.
• Enhancement: permit component inheritance through specifying EXTRA_COMPONENTS in config. See issue #32 https://github.com/ev3dev/brickstrap/issues/32
• Tighten validation of project directories. With the move to a unified projects/ hierarchy for default/example projects shipped with the brickstrap code, the validation logic for the -p switch needs a corresponding update to reflect this.

[cmacq2]

Rebased to latest master.

[dlech]

I'm getting [ fail] Unable to locate a readable 'multistrap.conf' when running

.../brickstrap.sh -p ev3dev-jessie -c base -c debian -c ev3 -d ev3 delete

Presumably this has something to do with the change in brp_iterate_components

[dlech]

Hmm... It's not even entering the br_list_paths function at line 286 in brickstrap.sh

   br_list_paths "multistrap.conf" -r >/dev/null || \
    fail "Unable to locate a readable 'multistrap.conf'"

[dlech]

Nevermind, the >/dev/null is hiding debug statements

[cmacq2]

This should be fixed now. Also moved the check until after we load config, so multistrap.conf can be inherited now.

[dlech]

Working for me now. One more request. Instead of EXTRA_COMPONENTS, could we call it INCLUDE_COMPONENTS?

[dlech]

I also tried adding the line EXTRA_COMPONENTS="debian base" to ev3dev-jessie/ev3/config and it does not seem to including the extra components.

[dlech]

It looks like the multistrap.conf is combined from the EXTRA_COMONENTS, so this code is probably working as designed. Must have uncovered another bug.

[dlech]

OK. I think the config files are not being combined properly when using EXTRA_COMPONENTS. Maybe includes/extra components should be in a separate file instead of as a variable in config?

[cmacq2]

OK. I think the config files are not being combined properly when using EXTRA_COMPONENTS. Maybe includes/extra components should be in a separate file instead of as a variable in config?

No that is by design. The design is a single top level config pulls in EXTRA_COMPONENTS it depends on.

So you cannot import additional config files. Imported components also cannot include additional EXTRA_COMPONENTS for that reason.

---

And I'm not sure we should attempt to 'fix' that either. Right now we can be reasonably certain that we catch invalid EXTRA_COMPONENTS and because we don't do recursive imports we also do not need to track which components were imported thus far beyond the global list of EXTRA_COMPONENTS.

... Why exactly would you need to import extra config files anyway? Just how granular do you split your settings, given that the user is supposed to select feature components directly via the -c switch?

[dlech]

Why exactly would you need to import extra config files anyway?

So that there is not duplicated configuration in each top-level component.

My component tree is: (<component>: <depends on>)

# board configurations - these are the 4 top-level components
ev3: base debian
evb: base debian
rpi1: rpi-base raspbian
rpi2: rpi-base debian
# internal components that cannot be arbitrarily used with other components
rpi-base: base
base:
debian: 
raspbian:

I plan to have other components later that can be arbitrarily mixed in. But there are special rules that have to be followed for the existing components. I am looking to hide these details so you don't have to look at the readme every time to figure out which 3 (or 4) components you have to use to get a valid image.

So, I have base/config that has the items shared by all boards, and debian/config and raspbian/config to sort out differences in the package sources and finally ev3/config and rpi-base/config because I have a mix of armel and armhf. To use EXTRA_COMPONENTS, I also have to create a new rpi1/config and rpi2/config. Basically, I have a reason to have a config in every component.

[cmacq2]

Well we could have a include file that is meant for the EXTRA_COMPONENTS setting specifically. I'll adjust the PR accordingly and I'll rename the variable to INCLUDE_COMPONENTS.

As for how your components inherit other components: that's inherently recursive in nature. I'll need to rework the PR a bit for that.

Oddly enough: at least two of your config files are basically what a previous PR of mine wanted to introduce the BR_ARCH variable (a switch with Debian architecture name) for, though I suppose the difference is that you want to hide the detail of the architecture instead of providing a template config that just needs the architecture 'plugged in'...

[cmacq2]

I redid the import/EXTRA_COMPONENTS logic.

Now you can create an include file which works similar to package files and lists additional components to import line-by-line (one component per line).

Recursive imports are supported.

[dlech]

Running

.../brickstrap.sh -p ev3dev-jessie -c ev3 -d ev3 create-rootfs

results in

[ fail] At least one component is required

[cmacq2]

I've added a fix for missing/empty /etc/shells in the rootfs which causes add-shell from debianutils version 4.6 to fail. Without the fix dash, bash and other login shells aren't properly configured as part of dpkg --configure -a.

Additionally it appears that the 'run dpkg --configure twice' thing is no longer required so I removed it as well as the || true bit.

[cmacq2]

I've added another fix/improvement:

• The gawk package is technically an optional Debian package so brickstrap should not hard depend on the user install it as part of the multistrap. By contrast mawk is a required package, hence it should be available and can be used as fallback to provide a usable awk.
• Additionally it's possible to move the 'awk' symlink to the 'private' /brickstrap tree inside the chroot.

The latest update to the PR adds both of these improvements.

[dlech]

This is too many unrelated changes for a single commit. Please split this up into one commit for each feature (and preferably make a separate pull request for each).

[cmacq2]

Hmm, I suppose you are right. It started out a bit torn between "fix" and "feature" and the additional fixes don't make it any more focused. I'll try and split this up.

... In the meantime, could you continue your review/testing of the actual code? (So I can incorporate results directly.)

[cmacq2]

I've created pull requests 41 thru 45 to isolate the bug fix/cleanup aspects of this PR.

The component import/inheriting feature has not been extracted yet: I expect it will create merge conflicts with the other PRs (due to spanning multiple files and offsets changing as a result of other PRs).

It would be cleaner to merge/reject the fixes first and then rebase the import feature on top of the new state of the master branch.

Hence, this PR should be kept open for now (until a new PR for the import feature is created).

[dlech]

The changes here seem to be working as advertised now.

[cmacq2]

I've extracted the component import/inheritance feature and prepared it as a separate PR. As a result all changes have corresponding PRs of their own and so this PR can now be closed.