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

Fetch particular git hash remotely in pi-gen releases #442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mahsank
Copy link

@mahsank mahsank commented Sep 3, 2020

GIT_HASH is not calculated properly in pi-gen releases as there is no git setup in releases archives. This commit fetches the release's corresponding commit hash remotely from pi-gen repo.

@XECDesign
Copy link
Member

Probably makes more sense to remove that line or set it to some text saying it's not from a git repo. People who forked the repo wouldn't want that URL hard-coded in there and there's nothing to say that what's on github matches their local files in any way.

@mahsank
Copy link
Author

mahsank commented Sep 3, 2020

Probably, it won't be a good idea to remove the line as in build-docker.sh GIT_HASH is used as an environment variable(?). One option is to generate the dummy commit hash and assign it to GIT_HASH. else block will look something like:

GIT_HASH=$(echo "GIT_HASH" | sha1sum | head -c 40)

This avoids hardcoding the URL. If this deems suitable, will modify the commit accordingly.

@XECDesign
Copy link
Member

build-docker is just passing it on so that it's not lost in the container. If it ends up being unset, it should end up being handled the same way.

It would be good to avoid something that looks like a valid git hash if it's not. Something like 'local' could be used instead?

@mahsank
Copy link
Author

mahsank commented Sep 4, 2020

build.sh already contains the PI_GEN_REPO that can be reused. build-docker.sh doesn't; just reusing the logic of build.sh in build-docker.sh. Updated the PR.

@XECDesign
Copy link
Member

I don't quite agree with the logic that the git hash will match what it gets from the remote tags, but if it's good enough for your use case, I have no problem with it.

Happy for it to be merged, or would you like to make any changes?

@mahsank
Copy link
Author

mahsank commented Sep 4, 2020

git hash ends up in /etc/rpi-issue in the final generated image. Pasted below are the contents of /etc/rpi-issue after generating a raspbian lite image from the release tag 2020-02-13-raspbian-buster with the patch applied.

pi@raspberrypi $ cat /etc/rpi-issue 
Raspberry Pi reference 2020-09-04
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 5f884374b6ac6e155330c58caa1fb7249b8badf1, stage2

Without, the patch, /etc/rpi-issue will look like:

pi@raspberrypi $ cat /etc/rpi-issue 
Raspberry Pi reference 2020-09-04
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, , stage2

The abovementioned patched image is generated wth build.sh. I am currently verifying that the hash ends up at the correct place in /etc/rpi-issue with build-docker.sh as well. Once, I verify that as well, it should be good to go.

@mahsank
Copy link
Author

mahsank commented Sep 7, 2020

This should be ok now.

@XECDesign
Copy link
Member

Couple of shellcheck issues:

In build-docker.sh line 60:
if [ -d $DIR/.git ]; then
        ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ -d "$DIR"/.git ]; then


In build-docker.sh line 63:
    GIT_HASH=$(git ls-remote --tags ${PI_GEN_REPO} | grep $(basename $PWD | cut -d'-' -f3-) | awk '{print $1}')
                                    ^------------^ SC2086: Double quote to prevent globbing and word splitting.
                                                          ^-- SC2046: Quote this to prevent word splitting.
                                                                     ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    GIT_HASH=$(git ls-remote --tags "${PI_GEN_REPO}" | grep $(basename "$PWD" | cut -d'-' -f3-) | awk '{print $1}')


In build.sh line 182:
if [ -z ${GIT_HASH} ]; then
        ^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ -z "${GIT_HASH}" ]; then


In build.sh line 183:
    if [ -d $(dirname $PWD)/.git ]; then
            ^-------------^ SC2046: Quote this to prevent word splitting.
                      ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ -d $(dirname "$PWD")/.git ]; then


In build.sh line 186:
        export GIT_HASH=$(git ls-remote --tags ${PI_GEN_REPO} | grep $(basename $PWD | cut -d'-' -f3-) | awk '{print $1}')
               ^------^ SC2155: Declare and assign separately to avoid masking return values.
                                               ^------------^ SC2086: Double quote to prevent globbing and word splitting.
                                                                     ^-- SC2046: Quote this to prevent word splitting.
                                                                                ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        export GIT_HASH=$(git ls-remote --tags "${PI_GEN_REPO}" | grep $(basename "$PWD" | cut -d'-' -f3-) | awk '{print $1}')

Unless I missed something, ${BASE_DIR} should be used instead of $(dirname "$PWD") in build.sh

@mahsank
Copy link
Author

mahsank commented Sep 29, 2020

Hopefully, it would be fine this time.

@mahsank
Copy link
Author

mahsank commented Oct 20, 2020

Any updates on this?

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