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

Run an init script in the container builds to set git email #3045

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

awood
Copy link
Contributor

@awood awood commented Feb 8, 2024

This is a result of the update of
com.netflix.nebula:nebula-release-plugin from 18.0.8 to 19.0.4. For
some reason the new plugin demands that the user.name and user.email be
set in the git configuration.

Copy link
Contributor

@Sgitario Sgitario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to do this? Theoritically, the only needed changes was done in #3021 and the build of the images was fixed.

@san7ket
Copy link
Contributor

san7ket commented Feb 9, 2024

/retest

@Sgitario
Copy link
Contributor

Sgitario commented Feb 9, 2024

Update: QE just noticed that the deployment to stage was failing. I fixed that using this commit 50a0a98.
The solution is the same as in the linked pull request: #3021.

Did you encounter this issue in somewhere else?

@awood
Copy link
Contributor Author

awood commented Feb 9, 2024

Yes, you can't build the images by hand from the Dockerfiles without this. So the bin/build-images.sh script that can be used for building images to deploy into the ephemeral environments by developers was broken.

@Sgitario
Copy link
Contributor

Yes, you can't build the images by hand from the Dockerfiles without this. So the bin/build-images.sh script that can be used for building images to deploy into the ephemeral environments by developers was broken.

Developers should have configured their user name and email locally. Actually, I didn't hit the issue when I tried to run this script. The problem with these changes is that we will always use the user of the last commit.

@awood
Copy link
Contributor Author

awood commented Feb 12, 2024

Developers should have configured their user name and email locally. Actually, I didn't hit the issue when I tried to run this script. The problem with these changes is that we will always use the user of the last commit.

Yes, I agree. I do actually have the name and email configured locally. The problem is that I had that information configured in ~/.gitconfig. Consequently, those values were not in .git/config in the project. I made the change to add the value to the local config, but this is a bit of a landmine. I can see it blowing up in someone else's face in the future (because after all, why bother setting your name and email for each repo when you can just do it globally?).

Any thoughts on the best way to warn people about this?

@Sgitario
Copy link
Contributor

Developers should have configured their user name and email locally. Actually, I didn't hit the issue when I tried to run this script. The problem with these changes is that we will always use the user of the last commit.

Yes, I agree. I do actually have the name and email configured locally. The problem is that I had that information configured in ~/.gitconfig. Consequently, those values were not in .git/config in the project. I made the change to add the value to the local config, but this is a bit of a landmine. I can see it blowing up in someone else's face in the future (because after all, why bother setting your name and email for each repo when you can just do it globally?).

Any thoughts on the best way to warn people about this?

Indeed, this is a very annoying change by the Nebula release plugin (why are we using this plugin? :S)

To be honest, since we're not really using the GIT user for anything, I would be ok to add the following:

# Initialize the GIT config which is required by the Gradle Nebula plugin to build the images
git config user.name "$(git --no-pager log --format=format:'%an' -n 1)"
git config user.email "$(git --no-pager log --format=format:'%ae' -n 1)"

as part of the build-images.sh script as done also in 50a0a98.

This is mostly to avoid having to modify all the Dockerfile (and hence increasing the complexity of those) to deal with a suspicious change of the Nebula plugin.

@awood
Copy link
Contributor Author

awood commented Feb 12, 2024

To be honest, since we're not really using the GIT user for anything, I would be ok to add the following:

# Initialize the GIT config which is required by the Gradle Nebula plugin to build the images
git config user.name "$(git --no-pager log --format=format:'%an' -n 1)"
git config user.email "$(git --no-pager log --format=format:'%ae' -n 1)"

as part of the build-images.sh script as done also in 50a0a98.

I thought about that, but wouldn't that end up affecting the person's local repo? It'd end up setting the username and email in their local repo to whatever HEAD was. Maybe the easiest thing is just a note in the README.

@kahowell kahowell added the QE Unneeded Pull request does not need QE approval label Feb 13, 2024
@awood awood merged commit 96ca7e5 into main Feb 13, 2024
3 of 5 checks passed
@awood awood deleted the awood/fix-dockerfiles branch February 13, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QE Unneeded Pull request does not need QE approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants