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

Fixes #5773: guard against user's ~/.gitconfig setting init.templateDir #5774

Merged
merged 6 commits into from
Jul 16, 2022
Merged

Fixes #5773: guard against user's ~/.gitconfig setting init.templateDir #5774

merged 6 commits into from
Jul 16, 2022

Conversation

mhucka
Copy link
Contributor

@mhucka mhucka commented Jul 15, 2022

This is a fix for issue #5773. The approach involves passing an explicit --template parameter to the git init commands in dev_tools/bash_scripts_test.py. This will override any settings that the user happens to have for init.templateDir in their ~/.gitconfig file.

If the user has a ~/.gitconfig file where init.templateDir is set,
then some of the tests in bash_scripts_test.py may fail because the
settings may cause the commands in the script to produce output that
doesn't match expectations. The changes here pass an explicit argument
to `git init` commands to override anything the user may have set in
their ~/.gitconfig file.
@mhucka mhucka requested review from a team, vtomole and cduck as code owners July 15, 2022 04:18
@CirqBot CirqBot added the Size: XS <10 lines changed label Jul 15, 2022
@mhucka mhucka changed the title Fix #5773: guard against user's ~/.gitconfig setting init.templateDir Fixes #5773: guard against user's ~/.gitconfig setting init.templateDir Jul 15, 2022
Comment on lines 62 to 63
mkdir empty-templates
git init --template=empty-templates --quiet --initial-branch master
Copy link
Collaborator

@pavoljuhas pavoljuhas Jul 15, 2022

Choose a reason for hiding this comment

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

Can you try if inserting

export GIT_CONFIG_GLOBAL=/dev/null
export GIT_CONFIG_SYSTEM=/dev/null

at the start of the cmd would fix the issue as well?

This should make git run with its defaults, regardless of system and user configuration files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested this, and you're right, it works too. This is a better solution because it will prevent other possible user settings from interfering, not just the one for template dir. I'll edit the PR.

This solves the problem in a different way, using the solution
suggested by @pavoljuhas in comments on the PR. The new approach uses
environment variables GIT_CONFIG_SYSTEM and GIT_CONFIG_GLOBAL to
override per-user configuration in ~/.gitconfig. This is a better
solution because it makes git completely ignore the user's
configuration, rather than only addressing the single template dir
problem of my original solution.
Comment on lines 322 to 323
'export GIT_CONFIG_GLOBAL=/dev/null\n'
'export GIT_CONFIG_SYSTEM=/dev/null\n'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed, because the setup argument for the run() function above
is inserted after the exports on lines 60-61.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, foo. I didn't notice how run was being used. Serves me right for trying to do things too quickly. Thanks for noticing and explaining that.

Turns out doing the settings of GIT_CONFIG_GLOBAL and
GIT_CONFIG_SYSTEM around line 322-323 is unnecessary because the
function `run()` is being applied here too, and we've already added
the settings on online 60-61.
@mhucka
Copy link
Contributor Author

mhucka commented Jul 15, 2022

One of the CI checks failed, but as far as I can tell, it's due to some timeout. Unfortunately, I don't seem to have the privileges to re-run the test (presumably due to team membership/github permissions—which is understandable and I'm not asking for a change there) so can't cause it to retry.

@pavoljuhas
Copy link
Collaborator

I just updated the branch which should rerun the CI checks again.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM

@pavoljuhas pavoljuhas added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 16, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 16, 2022
@CirqBot CirqBot merged commit f3987d5 into quantumlib:master Jul 16, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 16, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…nit.templateDir` (quantumlib#5774)

This is a fix for issue quantumlib#5773. The approach involves passing an explicit `--template` parameter to the `git init` commands in `dev_tools/bash_scripts_test.py`. This will override any settings that the user happens to have for `init.templateDir` in their `~/.gitconfig` file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: XS <10 lines changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants