checkout
checkout copied to clipboard
`safe.directory` handling broken when using Cygwin Git
The fix at f25a3a9 is great for the vast majority of folk, but the way safe.directory
is set and handled breaks my use case: I'm using Cygwin Git on Windows runners (I'm the maintainer of a number of Cygwin packages, including the Cygwin Git package). I'm installing Cygwin Git on the runner and adding it to the PATH before calling actions/checkout, which means actions/checkout uses Cygwin Git rather than the default Git that's present on the runner.
Prior to Git v2.35.1, this worked perfectly. Using Git v2.35.1 or v2.35.2 with actions/[email protected], I needed to configure safe.directory
before calling actions/checkout, but that makes sense and worked. However, with actions/[email protected], safe.directory
gets overridden to (say) D:\a\Cygwin-Git\Cygwin-Git
, and that doesn't work, because that's not the sort of path Cygwin Git expects. Even setting safe.directory
to /cygdrive/d/a/Cygwin-Git/Cygwin-Git
in an earlier step doesn't work, because when git
gets called as part of actions/checkout, it doesn't look at the default global .gitconfig at all.
I'm not sure what the best solution is here; I'm aware my use case is fairly niche. Options I can see:
- Don't use actions/checkout, or at least maintain my own fork of actions/checkout that handles Cygwin gracefully. This'd be a shame, but I entirely understand that supporting non-default Git installations on the runners isn't what y'all signed up for.
- I work out how to get Cygwin Git to accept Windows-style paths to be handled in this configuration option, as well as the more POSIX-like Cygwin ones. There's already code in the Cygwin wrapper layers that handles this when the executable is called – which is why the call to
git init
works at all – but that layer is bypassed after the initial executable call. - Set
safe.directory
to*
rather than a specific path in this action. I think that's safe – certainly I can't immediately come up with something that would break, given the config would only exist for the duration of the action run – but it might not be in the spirit of that configuration option. - Add some configuration toggle to make this scenario work; default to the behaviour added in f25a3a9, but allowing someone using actions/checkout to disable the new behaviour.
- Something else cunning I haven't yet thought of.
For reference, the output I'm getting right now when trying to use actions/checkout looks like this:
Run actions/checkout@v2
Syncing repository: me-and/Cygwin-Git
Getting Git version info
Temporarily overriding HOME='D:\a\_temp\01a36204-8528-42[13](https://github.com/me-and/Cygwin-Git/runs/6030136677?check_suite_focus=true#step:4:13)-acd7-fa55cd902b9e' before making global git config changes
Adding working directory to the temporary git global config as a safe directory
C:\cygwin\bin\git.exe config --global --add safe.directory D:\a\Cygwin-Git\Cygwin-Git
Deleting the contents of 'D:\a\Cygwin-Git\Cygwin-Git'
Initializing the repository
C:\cygwin\bin\git.exe init D:\a\Cygwin-Git\Cygwin-Git
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint:
hint: git config --global init.defaultBranch <name>
hint:
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint:
hint: git branch -m <name>
Initialized empty Git repository in /cygdrive/d/a/Cygwin-Git/Cygwin-Git/.git/
C:\cygwin\bin\git.exe remote add origin https://github.com/me-and/Cygwin-Git
Error: fatal: unsafe repository ('/cygdrive/d/a/Cygwin-Git/Cygwin-Git' is owned by someone else)
To add an exception for this directory, call:
git config --global --add safe.directory /cygdrive/d/a/Cygwin-Git/Cygwin-Git
Error: The process 'C:\cygwin\bin\git.exe' failed with exit code 1[28](https://github.com/me-and/Cygwin-Git/runs/6030136677?check_suite_focus=true#step:4:28)
Hey @me-and , thanks for the report!
Don't use actions/checkout, or at least maintain my own fork of actions/checkout that handles Cygwin gracefully. This'd be a shame, but I entirely understand that supporting non-default Git installations on the runners isn't what y'all signed up for.
I'd love to avoid this if we can
I don't fully understand how Cygwin works, but we copy the existing git global config from
const gitConfigPath = path.join(
process.env['HOME'] || os.homedir(),
'.gitconfig'
)
The safe.directory
option supports a list, so we just add another option to that list! It shouldn't overwrite anything in that list
So i'm curious, what is breaking this part:
because when git gets called as part of actions/checkout, it doesn't look at the default global .gitconfig at all.
Do we need to find the path to the default global config from git itself, rather than trying to process.env['HOME']
? Do you have a link to a run where you ran into this in a public repo? If not, I can try to spin up my own test
There's a run at https://github.com/me-and/Cygwin-Git/runs/6029980215 that shows this behaviour, although I had the impression that you wouldn't be able to view the logs even though it's a public repository. I can spin up a much simpler test case if that'd be useful, too.
IIRC, Windows systems typically don't have a HOME
environment variable set, so I think the current process looks something like this:
- I call Cygwin Git with
git config --global safe.directory ...
. At this point there is noHOME
environment variable, so Cygwin uses its default, which is effectively something likeC:\cygwin\home\username
. - The code you've quoted doesn't use
process.env['HOME']
, because that's not set, so it instead falls back toos.homedir()
, which will be something likeC:\Users\username
. So it's looking at a different.gitconfig
file to the one I've created. - When Cygwin Git loads as part of this action, at that point there is a
HOME
environment variable, and Cygwin will respect that if it's there, so it also doesn't see the config file I specified in the first step.
(I've not actually tested the above, but it seems to make sense to me at least, and does explain the symptoms.)
This has pointed me at a couple of other possible workarounds, though:
- I can set
safe.directory
in the system Git config file, which I'd half-forgotten existed until now. That'll be at something likeC:\cygwin\etc\gitconfig
(which Cygwin applications would see as/etc/gitconfig
), and would be read regardless of whatHOME
is (or isn't) set to. - I can export
HOME=C:\cygwin\home\username
into the runner environment before calling the action, at which point the action should pick up the correct config file to use and any config I've already set in it.
I'm not sure that getting Git to give you the correct path to the config file to use will actually help here, unfortunately, at least without a lot more care: if you ask Cygwin Git where it thinks the global config file lives, it'll output something like /home/user/.gitconfig
, which will be correct from the perspective of anything running within a Cygwin environment, but isn't meaningful to anything that's expecting to handle Windows paths. There are solutions to that – Cygwin provides a cygpath
utility precisely for converting between Windows and Cygwin paths – but I don't think this action should be doing things that relies on handling that level of environment-specific detail.
That said, rather than getting Git to tell you the path to the config file, you could just get it to dump out the contents of the config file, which should be much less fragile. It's a bit hacky, but you could use git -c core.editor=cat config --global -e
to drop the current global user config file onto stdout, then use that as the basis for the new temporary config file.
- Set
safe.directory
to*
rather than a specific path in this action. I think that's safe – certainly I can't immediately come up with something that would break, given the config would only exist for the duration of the action run – but it might not be in the spirit of that configuration option.
This is a useful trick, thanks! (Note that whatever solution you all go for, it should handle submodules gracefully; see also https://github.com/actions/checkout/issues/766#issuecomment-1100104891)
Set safe.directory to * rather than a specific path in this action. I think that's safe...
Unfortunately, there's a window of git versions that have the new safe.directory
handling but don't understand *
. 😢
Maybe both safe.directory
to the specified path and safe.directory=*
. 🤔 That way, users who have a new enough version of git for it to support safe.directory=*
are covered, and users who have the security release that understands safe.directory
(but not the wildcard) still get the current behavior.
Maybe both
safe.directory
to the specified path andsafe.directory=*
. 🤔 That way, users who have a new enough version of git for it to supportsafe.directory=*
are covered, and users who have the security release that understandssafe.directory
(but not the wildcard) still get the current behavior.
That definitely works for anything using current Cygwin releases, doesn't sound like it'd break anything for anyone else, and while I can contrive Cygwin-based edge cases where it wouldn't work, they're very contrived – either someone self-building one of the Git releases that has safe.directory
but not safe.directory=*
, or going out of their way to install such a release, then adding that build to the PATH
and running this action.
There's a purist part of me that doesn't like the idea of having redundant configuration lines for Git versions that support both options, and therefore either wants to test what option should be used every time or to make it something that users of this action could toggle with a with:
option. But I don't think that's actually sensible, and just configuring both options regardless seems much more pragmatic.
What about we change actions/checkout
to not set safe.directory
if your global .gitconfig
already have it?
@TingluoHuang I think that depends on how you check. Running git config --global safe.directory
and checking if the output is non-empty would definitely work. Checking the contents of the .gitconfig
file won't help for the same reasons the current solution doesn't work: the Windows runners don't have the HOME
environment variable defined, meaning actions/checkout
looks for the global .gitconfig
in a different place to where Cygwin Git expects it to be.
For now, per https://github.com/me-and/Cygwin-Git/commit/08056b45533326523201414990bbc8968025331a, I'm working around this by just defining HOME
explicitly, and that seems to have got everything working.
@me-and I am planning to merge https://github.com/actions/checkout/pull/770 which allows you to turn off the safe.directory
setting we write.