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

Inconsistencies using git describe --dirty #250

Closed
umarcor opened this issue May 18, 2020 · 7 comments
Closed

Inconsistencies using git describe --dirty #250

umarcor opened this issue May 18, 2020 · 7 comments

Comments

@umarcor
Copy link

umarcor commented May 18, 2020

While trying to use git describe --dirty to get a description of the repo, I found the behaviour of v2 misleading. At the same time, there are certain setups that produce a "dirty" state on Windows. To test it, I configured ~40 jobs in https://github.com/umarcor/ghdl/blob/checkout-dirty/.github/workflows/checkout.yml. Results are summarized in the following table:

job actions unshallow depth 0 inherit clone in MSYS2 pacman git dirty? msys dirty?
v1 (ubuntu) v1 - - - - - no -
v1 (windows) v1 - - - - - no -
v1 (macos) v1 - - - - - no -
v2 (ubuntu) v2 - - - - - fatal: No names found, cannot describe anything -
v2 (windows) v2 - - - - - fatal: No names found, cannot describe anything -
v2 (macos) v2 - - - - - fatal: No names found, cannot describe anything -
v2-depth0 (ubuntu) v2 - x - - - fatal: No names found, cannot describe anything no -
v2-depth0 (windows) v2 - x - - - fatal: No names found, cannot describe anything no -
v2-depth0 (macos) v2 - x - - - fatal: No names found, cannot describe anything no -
v2-unshallow (ubuntu) v2 x - - - - no -
v2-unshallow (windows) v2 x - - - - no -
v2-unshallow (macos) v2 x - - - - no -
v2-depth0-unshallow (ubuntu) v2 x x - - - fatal: --unshallow on a complete repository does not make sense -
v2-depth0-unshallow (windows) v2 x x - - - fatal: --unshallow on a complete repository does not make sense -
v2-depth0-unshallow (macos) v2 x x - - - fatal: --unshallow on a complete repository does not make sense -
master (ubuntu) master - - - - - fatal: No names found, cannot describe anything -
master (windows) master - - - - - fatal: No names found, cannot describe anything -
master (macos) master - - - - - fatal: No names found, cannot describe anything -
master-depth0 (ubuntu) master - x - - - fatal: No names found, cannot describe anything no -
master-depth0 (windows) master - x - - - fatal: No names found, cannot describe anything no -
master-depth0 (macos) master - x - - - fatal: No names found, cannot describe anything no -
master-unshallow (ubuntu) master x - - - - no -
master-unshallow (windows) master x - - - - no -
master-unshallow (macos) master x - - - - no -
msys-depth0 (MINGW32) master - x - - - fatal: No names found, cannot describe anything no git: command not found
msys-depth0 (MINGW64) master - x - - - fatal: No names found, cannot describe anything no git: command not found
msys-unshallow (MINGW32) master x - - - - no git: command not found
msys-unshallow (MINGW64) master x - - - - no git: command not found
msys-depth0-git (MINGW32) master - x - - x fatal: No names found, cannot describe anything no yes
msys-depth0-git (MINGW64) master - x - - x fatal: No names found, cannot describe anything no yes
msys-unshallow-git (MINGW32) master x - - - x no yes
msys-unshallow-git (MINGW64) master x - - - x no yes
msys-depth0-inherit (MINGW32) master - x x - - - fatal: No names found, cannot describe anything no
msys-depth0-inherit (MINGW64) master - x x - - - fatal: No names found, cannot describe anything no
msys-unshallow-inherit (MINGW32) master x - x - - - no
msys-unshallow-inherit (MINGW64) master x - x - - - no
msys-clone (MINGW32) - - - - x x - no
msys-clone (MINGW64) - - - - x x - no
msys-inherit-clone (MINGW32) - - - x x - - no
msys-inherit-clone (MINGW64) - - - x x - - no

Overall, I find misleading that using fetch-depth: 0 does not retrieve tags and, at the same time, it does not allow using --unshallow. It seems not possible to use the Action as is to retrieve some describable state. Some additional step is always required.

In my target use case, I need to run git describe --dirty in a script that is executed on MSYS2 (both MINGW32 and MING64). As shown in the table, there seem to be two possible setups only, and one of them produces an invalid result.

Precisely, cases msys-unshallow-git produce a dirty description, even if the repo is clean. These cases imply cloning a repo with this action and the using git installed through pacman. It seems to be some clonflict with line endings, but neither dos2unix nor unix2dos can fis it easily.
Hence, in order to get a clean description, users are "forced" to inherit the PATH and to avoid using git installed through pacman (cases msys-unshallow-inherit). This is undesirable, because other tools might be overriden through the PATH.

Alternatively, manually cloning the repo inside MSYS2 works ok, regardless of using git inherited from the PATH (cases msys-inherit-clone) or installed through pacman (cases msys-clone). However, this defeats the purpose of this action.

/cc @ericsciple

Ref #249 #240 #239 #217 #226

@ericsciple
Copy link
Contributor

ericsciple commented May 27, 2020

@umarcor i updated v2 so that fetch-depth: 0 now fetches all branches and tags

@ericsciple
Copy link
Contributor

Let me know if that fixes the issue. Otherwise the default behavior for v2 persists the auth token, so scripting authenticated git commands just works . So if you need to run git fetch it should just work. Hope that helps, let me know. Also if it helps, you can detect whether shallow based upon whether .shallow file exists.

@umarcor
Copy link
Author

umarcor commented May 28, 2020

@ericsciple thanks! I believe that it won't make a big difference because our scripts do check it explicitly now. However, I will try it. Is this released already or is it in master yet?

So if you need to run git fetch it should just work.

Yes, it did/does. Precisely, our fix was: https://github.com/ghdl/ghdl/blob/master/dist/msys2-mingw/run.sh#L70-L74. Nevertheless, I believe that the new behaviour will be better for a majority of users.

Regarding the second issue (this action producing a dirty state when using git describe from MSYS2), I think it is specially relevant now that MSYS2 is included in windows-latest environments by default. For our use case, I found that using git config --global core.autocrlf input before this Action would fix the problem: https://github.com/ghdl/ghdl/blob/master/.github/workflows/push.yml#L72-L82. Note that eine/setup-msys2@v0 with update: true uses the default installation in C:\msys64. Hence, I'd suggest this Action to set autocrlf to input by default. Alternatively, autocrlf might be an option (#226).

@umarcor
Copy link
Author

umarcor commented May 28, 2020

@ericsciple, I ran the tests again: https://github.com/umarcor/ghdl/actions/runs/117853978. I updated the table above accordingly. As you said, the "unshallow" issue seems to be fixed. All *-depth0 jobs produce the same results as corresponding *-unshallow jobs 🎉.

However, msys-unshallow-git or msys-depth0-git jobs do still produce a dirty description. I.e., using this Action with depth: 0 and then executing git describe --dirty with the git available inside MSYS2. See https://github.com/umarcor/ghdl/runs/716524334?check_suite_focus=true

@ericsciple
Copy link
Contributor

For the crlf issue, there is a good discussion here

Based on this doc the core.autocrlf input produces "CRLF endings in Windows checkouts, but LF endings on macOS and Linux systems and in the repository". I'm confused why that would help, isnt CRLF on Windows the default behavior anyway? I'm a bit hesitant to change the default at this point and risk disrupting existing consumers of checkout@v2. Perhaps in a v3 would be a better time to change the default.

I agree a crlf input on the action sounds like it would help (tracked by this issue)

@umarcor
Copy link
Author

umarcor commented Jun 4, 2020

I'm confused why that would help, isnt CRLF on Windows the default behavior anyway?

input is equal to "just ignore the endings of the local files and always use LF". So, it is irrelevant if files are cloned as LF or CRLF, because any git status, git describe, etc. ignores the difference (and converts CRLF to LF before writing in the internal git database).

I'm a bit hesitant to change the default at this point and risk disrupting existing consumers of checkout@v2. Perhaps in a v3 would be a better time to change the default.

I understand that. In fact, I think that having this as an option (which can be added to v2) might suffice. Anyway, let's continue in #226.

@ericsciple
Copy link
Contributor

Aha thanks for the explanation of input. Makes sense to me now. Thanks!

karlhorky added a commit to upleveled/preflight that referenced this issue Feb 3, 2021
gruvector pushed a commit to gruvector/preflight that referenced this issue Feb 9, 2024
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

No branches or pull requests

2 participants