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

ci: avoid d/f conflict in vs/master #2618

Merged
merged 3 commits into from
May 9, 2020

Conversation

dscho
Copy link
Member

@dscho dscho commented May 9, 2020

As can be seen in this run, our current GitHub workflow does not play well with our vs/master branch: we commit .vcxproj files in directories named identically to the executables' names. This works on Windows because the executables actually have an additional .exe filename suffix. But on Linux/macOS, the compilation fails because they cannot write, say, git: there is already a directory under that name.

Let's fix this by renaming those directories, appending the suffix .proj. Oh, and work around .git/branches/ problems (why exactly do we still create this directory?).

It already caused problems with the test suite that the directory
containing `git.vcxproj` is called the same as the Git executable
without its file extension: `./git` is ambiguous, it could refer both to
the directory `git/` as well as to `git.exe`.

Now there is one more problem: when our GitHub workflow runs on the
`vs/master` branch, it fails in all but the Windows builds, as they want
to write the file `git` but there is already a directory in the way.

Let's just go ahead and append `.proj` to all of those directories, e.g.
`git.proj/` instead of `git/`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the avoid-d/f-conflict-in-vs/master branch from f22aed1 to 8b2d27b Compare May 9, 2020 16:20
Copy link

@PhilipOakley PhilipOakley left a comment

Choose a reason for hiding this comment

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

Looks good except for two white space changes.

t/t5516-fetch-push.sh Show resolved Hide resolved
t/t5505-remote.sh Show resolved Hide resolved
Copy link

@PhilipOakley PhilipOakley left a comment

Choose a reason for hiding this comment

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

The changes look & sound sensible, though I haven't tested them myself.

@PhilipOakley
Copy link

I think I recognise that run 😉. Thank you for working on this so quickly.

I think the .git/branches/ problem was from really distant Git history and that it was kept for some tentative backward compatibility issue. (learned from the mailing list, you probably knew the history - Ahh, seen the commit message comment!).

dscho added 2 commits May 9, 2020 19:30
When we commit the template directory as part of `make vcxproj`, the
`branches/` directory is not actually commited, as it is empty.

Two tests were not prepared for that situation.

This developer tried to get rid of the support for `.git/branches/` a
long time ago, but that effort did not bear fruit, so the best we can do
is work around in these here tests.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The convention in Git project's shell scripts is to have white-space
_before_, but not _after_ the `>` (or `<`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the avoid-d/f-conflict-in-vs/master branch from d2cc0c6 to 60fe2fd Compare May 9, 2020 17:31
@dscho
Copy link
Member Author

dscho commented May 9, 2020

I think the .git/branches/ problem was from really distant Git history

It actually stems from cogito... a long-defunct front-end to Git, back from the days when Linus still thought that Git would be almost exclusively plumbing, and everybody would write their own wrappers around it.

Maybe at some stage I will have learned enough diplomacy to get something like https://lore.kernel.org/git/cover.1494509599.git.johannes.schindelin@gmx.de/ off the ground. For now, I just have to live with .git/branches/ still being a thing.

@dscho
Copy link
Member Author

dscho commented May 9, 2020

BTW if you're interested to know why the .git/branches/ issue does not cause any problems in the vs-test job, here is the answer: https://lore.kernel.org/git/pull.287.v2.git.gitgitgadget@gmail.com/ (look for the "Range-diff vs v1:").

@dscho
Copy link
Member Author

dscho commented May 9, 2020

@PhilipOakley would you care to re-review the latest two commits?

Copy link

@PhilipOakley PhilipOakley left a comment

Choose a reason for hiding this comment

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

5a818a2 LGTM. (Not sure if the "approve" button would do too many things...)

Copy link

@PhilipOakley PhilipOakley left a comment

Choose a reason for hiding this comment

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

60fe2fd LGTM!

@dscho
Copy link
Member Author

dscho commented May 9, 2020

Not sure if the "approve" button would do too many things...

It wouldn't do too many things, it would instead record your LGTM in a more formal manner ;-)

@dscho
Copy link
Member Author

dscho commented May 9, 2020

I'll just go ahead and merge it ;-)

@dscho dscho merged commit 46ad3fc into git-for-windows:master May 9, 2020
@dscho dscho deleted the avoid-d/f-conflict-in-vs/master branch May 9, 2020 19:25
git-for-windows-ci pushed a commit that referenced this pull request May 9, 2020
git-for-windows-ci pushed a commit that referenced this pull request May 9, 2020
git-for-windows-ci pushed a commit that referenced this pull request May 9, 2020
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2020
git-for-windows-ci pushed a commit that referenced this pull request May 10, 2020
git-for-windows-ci pushed a commit that referenced this pull request May 11, 2020
git-for-windows-ci pushed a commit that referenced this pull request Sep 25, 2024
git-for-windows-ci pushed a commit that referenced this pull request Sep 26, 2024
git-for-windows-ci pushed a commit that referenced this pull request Sep 26, 2024
dscho added a commit that referenced this pull request Sep 26, 2024
dscho added a commit that referenced this pull request Sep 26, 2024
dscho added a commit that referenced this pull request Sep 26, 2024
dscho added a commit that referenced this pull request Sep 26, 2024
dscho added a commit that referenced this pull request Sep 27, 2024
dscho added a commit to dscho/git that referenced this pull request Oct 3, 2024
…-in-vs/master

ci: avoid d/f conflict in vs/master
git-for-windows-ci pushed a commit that referenced this pull request Oct 4, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 4, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 4, 2024
dscho added a commit to dscho/git that referenced this pull request Oct 8, 2024
…-in-vs/master

ci: avoid d/f conflict in vs/master
git-for-windows-ci pushed a commit that referenced this pull request Oct 8, 2024
dscho added a commit that referenced this pull request Oct 8, 2024
dscho added a commit that referenced this pull request Oct 8, 2024
dscho added a commit that referenced this pull request Oct 8, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 9, 2024
dscho added a commit that referenced this pull request Oct 9, 2024
dscho added a commit that referenced this pull request Oct 9, 2024
dscho added a commit that referenced this pull request Oct 10, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 10, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 10, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 11, 2024
git-for-windows-ci pushed a commit that referenced this pull request Oct 11, 2024
dscho added a commit that referenced this pull request Oct 11, 2024
dscho added a commit that referenced this pull request Oct 11, 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

Successfully merging this pull request may close these issues.

2 participants