ev3dev / brickstrap

Tool for bootstrapping Debian and creating bootable image files for embedded systems
MIT License
35 stars 26 forks source link

Enable importing components using a custom 'include' file. #49

Closed cmacq2 closed 8 years ago

cmacq2 commented 8 years ago

See issue #32: https://github.com/ev3dev/brickstrap/issues/32

This change enables recursive imports. brp_import_extra_components will iteratively import components listed in 'include' files. The 'include' file format is identical to the package file format, except that instead of packages component directories are listed.

As per request, this PR isolates and supersedes the corresponding change from PR #39

dlech commented 8 years ago

I'm getting a bunch of .../brickstrap-components.sh: line 94: br_is_component_imported: command not found errors because br_is_component_imported is not defined anywhere.

dlech commented 8 years ago

Ah. it's defined as brp_is_component_imported (private vs. public)

dlech commented 8 years ago

Same with brp_is_component_selected

dlech commented 8 years ago

Furthermore, recursive includes do not seem to be working. For example I have rpi1 including rpi-base and raspbian (in that order) and rpi-base includes base. I expect the components to be loaded in the order base, rpi-base, raspbian, rpi1 (i.e. post-order tree traversal).

cmacq2 commented 8 years ago

This is now fixed. The issue was that BRP_INCLUDES is (deliberately) not iterated over in the iterate components function. So I had the variables 'crossed' initially.

As for the order: it is a traversal of a directed graph that avoids cycling (using BRP_INCLUDES) so your order winds up being:

dlech commented 8 years ago

As for the order

Can we change this? It is backwards from how I have my components setup. They expect to be run in a certain order. For most parts it probably doesn't mater, but for a few, it does.

Also, it is counterintuitive to my programming experience. Includes are generally at the beginning of a source code file which result in a post-order evaluation of the includes. So, I think most people would expect post-order as well.

cmacq2 commented 8 years ago

Can we change this? It is backwards from how I have my components setup.

That would require changes to brp_read_include_file to 'descend' and become effectively (probably indirectly) recursive. I'm not sure that this is such a great idea, much the same way I was initially reluctant to support recursive imports.

Also, it is counterintuitive to my programming experience.

I'm not sure this is a great argument. Fundamentally the 'include' file is not at all like a PHP include() not even a PHP require_once() but rather more like a reference to a symbol from an external class in e.g. Java (or indeed PHP set up with autoloaders).

More precisely it is just a declarative list of dependencies and anything that does dependency resolution typically approaches the problem as a directed graph (not considering constraint solvers).

They expect to be run in a certain order.

That is always going to be fragile. Remember that when we set up the component based path scheme the key always was that components be idempotent. This is also a requirement for hooks, and has been since brickstrap first supported hooks (because of the sourcing).

For example if your user does select more than one component the order in which those components are selected is going to change the order in which your include files are processed. That is going to be the case no matter whether or not we change the actual include parsing mechanism to yield a "post-order" list of components because at each level the order in which components are specified still "trumps" everything else.

dlech commented 8 years ago

Remember that when we set up the component based path scheme the key always was that components be idempotent.

Obviously not. :smile:

I will just have to fix up my components.

cmacq2 commented 8 years ago

... We can also prepend to the BRP_EXTRA_COMPONENTS variable instead of append, then your order would become something like this:

You'll note that the top level 'rpi1' still comes before the imported components (because it is selected). That would require processing BRP_EXTRA_COMPONENTS before BR_COMPONENTS in the iterating logic. And then it would happen to match your desired order, but this "matching" can be broken for more complex set ups involving inter-dependent components.

... I'm not sure that's such a great change either and in any case it doesn't do anything for the "order was changed because the user typed the command differently" problem.

dlech commented 8 years ago

No, I think you are right. We need to just say that components can be included in any order (via the command line or include files)

dlech commented 8 years ago

Tested again. Not sure how this happened, but when I ran...

.../brickstrap.sh -p ev3dev-jessie -c rpi1 -d rpi1x all -f -x sshfs

...it copied root/ files from the rpi2 component. I've double-checked my include files and there is no way I am telling it to include the rpi2 component. It also does not seem to be including anything else from the rpi2 component (packages is the only other thing in the rpi2 component).

cmacq2 commented 8 years ago

Did you try to overwrite previous output of a run with rpi2 ? The copy root stage doesn't actually delete anything prior to copying so previous output that isn't overwritten is left intact...

What was the output of:

    debug "Selected components: $BR_COMPONENTS"
    debug "Inherited includes: $BRP_INCLUDES"
    debug "Imported components: $BRP_EXTRA_COMPONENTS"

?

dlech commented 8 years ago
david@freyr:~/work/junk/brickstrap$ LOG_LEVEL=4 ~/work/ev3dev-dpkg/brickstrap/brickstrap.sh -p ev3dev-jessie -c rpi1 -d rpi1x create-conf -f
23:59:44 [debug] cmd: create-conf
23:59:44 [debug] SCRIPT_PATH: /home/david/work/ev3dev-dpkg/brickstrap
23:59:44 [debug] Selected components: 'rpi1'
23:59:44 [debug] Inherited includes:  'rpi-base'
23:59:44 [debug] Imported components: 'rpi-base' 'raspbian' 'base'
23:59:44 [ info] loading: /home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/rpi-base/config
23:59:44 [ info] script completed successfully: '/home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/rpi-base/config'
23:59:44 [ info] loading: /home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/raspbian/config
23:59:44 [ info] script completed successfully: '/home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/raspbian/config'
23:59:44 [ info] loading: /home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/base/config
23:59:44 [ info] script completed successfully: '/home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie/base/config'
23:59:44 [ info] Creating multistrap configuration file...
23:59:44 [debug] br_project_dir: /home/david/work/ev3dev-dpkg/brickstrap/projects/ev3dev-jessie
23:59:45 [debug] creating /home/david/work/junk/brickstrap/rpi1x/multistrap.conf
cmacq2 commented 8 years ago

So the components that are actually iterated over are:

I don't see any rpi2 stuff in there...

So what happens if you delete the old folder and re-run in a clean environment? I ask because:

The copy root stage doesn't actually delete anything prior to copying so previous output that isn't overwritten is left intact...

dlech commented 8 years ago

I haven't run with the components in that order but running -c base -c rpi-base -c rasbian -c rpi1 works correctly (no rpi2 stuff). It will be a few days before I can test again.

cmacq2 commented 8 years ago

Well if your fixed up components are available somewhere (separate branch) I could check it out and test...

dlech commented 8 years ago

I've added a commit with my include files in the import-components branch.

cmacq2 commented 8 years ago

I think this is a bug in your code.

The files you refer to are not actually copied from rpi2.

The files are actually copied from the raspbian component instead, and just happen to have identical contents to those in rpi2 (i.e. left over duplicates). To fix this, just delete the root/etc/flash-kernel tree from your raspbian component (and check there aren't any other copies).

dlech commented 8 years ago

Well, I must be blind. I thought that is what the problem must be and checked for it but somehow didn't see it before.

I've corrected my problem, but now the -x option is not working. There is currently a bug in the fuse package that prevents it from installing, so I haven't run through a complete rpi build yet.