ev3dev / brickstrap

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

Integrate component based path layering #23

Closed cmacq2 closed 8 years ago

cmacq2 commented 8 years ago

Integrate component based path layering into the main brickstrap script itself.

With this change brickstrap can take advantage of component base path scheme discussed in issue #17 and implemented in PR #22. For details refer to: Discussion: https://github.com/ev3dev/brickstrap/issues/17 Implementation PR: https://github.com/ev3dev/brickstrap/pull/22

Additional changes:

This change does not integrate the architecture/QEMU support from PR #21

cmacq2 commented 8 years ago

Updated to integrate feedback. I've tested this with a fresh run of a project build (using 'all') to check that the function renaming didn't accidentally break things.

cmacq2 commented 8 years ago

Do you have any further comments? Or can the PR be merged in as it stands now?

dlech commented 8 years ago

I would like to actually test it myself first. Haven't got around to that yet.

cmacq2 commented 8 years ago

Okay, looking forward to test results! Tip: pass -c . if you haven't converted your project directories yet.

cmacq2 commented 8 years ago

I've tightened validation of project & component names. This change disallows the following 'project' paths:

Additionally trivially duplicate component names are now ignored, and component names which attempt to 'escape' the project directory like ../ are disallowed.

There's still the edge case of someone specifying a non-trivial duplicate component name, for example: -c name1/../name2 -c name2

cmacq2 commented 8 years ago

Bug fix: match scripts against whole lines from preinst blacklist; substring matches should no longer lead to false positives.

Second attempt: turns out grep supports the anchors anyway, and using plain grep avoids escaping issues w.r.t. regex syntax.

cmacq2 commented 8 years ago

Updated to use the fgrep -xq "<pattern>" construct. This is cleaner than the previous 'fixes'..

dlech commented 8 years ago

Shouldn't we be using fgrep everywhere else we are grepping a variable string?

cmacq2 commented 8 years ago

Unless we explicitly use the anchors... fgrep/grep -F doesn't do those. I'll prepare a new update. I checked: it's just brickstrap.sh that needs to be adjusted.

dlech commented 8 years ago

Thanks for fixing up the greps. I'm running it one more time to make sure that changing the greps didn't break anything. In the meantime, the readme needs to be updated for the command line switch changes.

cmacq2 commented 8 years ago

I'd like to postpone the readme until we've also integrated the architecture/QEMU related changes into brickstrap.sh.

Also: if you are going to develop issue #24 further that will require additional matching readme updates as well.

dlech commented 8 years ago

Thanks!