Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix export-image to not depend on fake-hwclock being installed #326

Merged
merged 2 commits into from
Aug 27, 2019

Conversation

rkubes
Copy link
Contributor

@rkubes rkubes commented Aug 27, 2019

Currently the export-image/04-finalise/01-run.sh script depends on fake-hwclock being installed, since it calls /etc/init.d/fake-hwclock stop early on.
Since it's run under bash -e the build fails if fake-hwclock is not installed.
This is not an issue for the majority of users, but it prevents images from being built prior to stage2, and would impact any customization that removes fake-hwclock prior to export-image.

I could have done this by checking if the /etc/init.d/fake-hwclock file exists before executing it; however, that seemed a little extra rather than just adding the " || true" at the end of the command to force a 0 exit code. The risk with my method is if fake-hwclock is actually installed, but for some reason fails to stop, then the export-image script will continue anyway. My opinion is that risk is minimal, but I can change the pull request if desired.

@rkubes
Copy link
Contributor Author

rkubes commented Aug 27, 2019

Oops, I posted this too early. It still needs more work, since I missed the next line hardlink as a dependency.
I may need guidance on whether I'm going down a rabbit hole and the export-image stage should really be considered dependent on stage2.
I know generally builds off of stage1 are not useful, and my concern is potentially adding risk to the 99.99% of builds to satisfy the 0.01%

@rkubes
Copy link
Contributor Author

rkubes commented Aug 27, 2019

For what it's worth. When using both commits to remove the dependencies on fake-hwclock and hardlink, I can successfully export an image based off stage1.
I'm fine with either decision to merge these in to support even more minimalist builds, or if it's decided to close this without merging due to the fact that the majority of builds will have fake-hwclock and hardlink installed.

@XECDesign
Copy link
Member

Looks good, but maybe an if [ -x /usr/bin... ]; then type check is better to avoid unnecessary error being shown in the output?

@rkubes
Copy link
Contributor Author

rkubes commented Aug 27, 2019

Valid point. I made the changes. For checking hardlink I'm using hash instead of checking the absolute path, in case the user installs it to some other location.
I'm going to kick off a build to test this to ensure it works correctly, I haven't tested the change yet.

@XECDesign
Copy link
Member

Thank you, much appreciated.

@rkubes
Copy link
Contributor Author

rkubes commented Aug 27, 2019

I ran a build that exported an image from stage2 and a separate build that exported an image from stage1 using this change. Everything is working as expected. The stage1 build skips the two commands and doesn't display error messages. The stage2 build successfully runs the two commands, since they're present.

@XECDesign XECDesign merged commit 00c22ab into RPi-Distro:master Aug 27, 2019
mikehash added a commit to mikehash/pi-gen that referenced this pull request Aug 29, 2019
Updated export-image to not depend on fake-hwclock and hardlink. (RPi-Distro#326)
jriguera added a commit to jriguera/raspbian-cloud that referenced this pull request Nov 9, 2019
PeterJohnson pushed a commit to PeterJohnson/WPILibPi that referenced this pull request Dec 7, 2019
alexgg pushed a commit to balena-os/pi-gen that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants