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

De-symlink .gitignore to mollify new Git. #217

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

schmonz
Copy link
Member

@schmonz schmonz commented Aug 6, 2021

De-symlink .gitignore to mollify new Git.

In Git 2.32.0 and newer, git status (for instance) says:

warning: unable to access '.gitignore': Too many levels of symbolic links

https://github.com/git/git/blob/master/Documentation/RelNotes/2.32.0.txt
explains thus:

* It does not make sense to make ".gitattributes", ".gitignore" and
  ".mailmap" symlinks, as they are supposed to be usable from the
  object store (think: bare repositories where HEAD:.mailmap etc.
  are used). When these files are symbolic links, we used to read
  the contents of the files pointed by them by mistake, which has
  been corrected.

To satisfy Git, turn .gitignore back into a regular file containing the
contents of TARGETS. To satisfy other uses of TARGETS (such as in
exported tar archives, which don't include .gitignore), keep it as a
regular in-tree file too, with reminders in each to sync with the other.

@schmonz schmonz added the build label Aug 6, 2021
@schmonz schmonz added this to the 1.09 milestone Aug 6, 2021
@DerDakon
Copy link
Member

DerDakon commented Aug 6, 2021

This will break the generated tar archives as .git* is ignored in .gitattributes.

@schmonz schmonz force-pushed the schmonz-git2.32.0-gitignore-symlink-switch branch from 31fbf95 to 3b80887 Compare August 6, 2021 11:35
@schmonz schmonz changed the title Swap .gitignore and TARGETS to mollify new Git. De-symlink .gitignore to mollify new Git. Aug 6, 2021
@DerDakon
Copy link
Member

DerDakon commented Aug 6, 2021

And now we are autogenerating targets. Fine with me, but maybe we can take the step and clean up this ancient construct. .gitignore may just filter *.o, while TARGETS could simply be generated by grepping the objects from Makefile if it is generated anyway. Similar things may happen for other targets. If we open that door we should walk through it, not only peek inside.

@schmonz
Copy link
Member Author

schmonz commented Aug 6, 2021

Agree with going further, but only after the magical inflection point where people no longer need to apply the major patches so we can willingly break whatever minor patches they may have left.

@DerDakon
Copy link
Member

DerDakon commented Aug 6, 2021

I agree with that for TARGETS to a degree (see below), but for .gitignore we are not bound to anything.

TARGETS is just historic debt in my eyes. The only thing it is needed for is make clean, which isn't that important anyway. And if we are going to extract the target list from make it into a variable, and use that for both make it and make clean I doubt we have much left beyond *.o and *.a. And then we could tell people to simply ignore any failures in TARGETS.

But this is indeed the next step, which we could am for 1.10 or something like that, and now just make things work again. And for the time being let's just keep the files identical, i.e. without the "autogenerated" header. This way we don't needlessly duplicate things in git, noone cares if that is autogenerated in a release tarball (and there it is not even true not to touch it), and eventually we can just kill the whole thing with fire.

@schmonz schmonz force-pushed the schmonz-git2.32.0-gitignore-symlink-switch branch from 3b80887 to 7342b4a Compare August 6, 2021 21:23
@DerDakon
Copy link
Member

DerDakon commented Aug 7, 2021

I fear this will become endless… sorry for that.

Have you tried this with a release tarball? I bet not, and I have neither ;)

But now TARGETS is part of it. TARGETS is there, make will try to refresh it, but there is no rule to make .gitignore, required from TARGETS or something like that. As I said, I have not tried, but I bet this will happen.

Maybe we should just add a make tarball or something that includes the git archive … command from the wiki, and which depends on TARGETS, and will error out if git status is not clean afterwards. And when we are at it, maybe add make TARGETS and a git status check to CI so we can easily catch when someone forgets it.

@schmonz schmonz force-pushed the schmonz-git2.32.0-gitignore-symlink-switch branch from 60a2c70 to bdcdd92 Compare August 9, 2021 18:22
In Git 2.32.0 and newer, `git status` (for instance) says:

    warning: unable to access '.gitignore': Too many levels of symbolic links

https://github.com/git/git/blob/master/Documentation/RelNotes/2.32.0.txt
explains thus:

    * It does not make sense to make ".gitattributes", ".gitignore" and
      ".mailmap" symlinks, as they are supposed to be usable from the
      object store (think: bare repositories where HEAD:.mailmap etc.
      are used). When these files are symbolic links, we used to read
      the contents of the files pointed by them by mistake, which has
      been corrected.

To satisfy Git, turn .gitignore back into a regular file containing the
contents of TARGETS. To satisfy other uses of TARGETS (such as in
exported tar archives, which don't include .gitignore), keep it as a
regular in-tree file too, with reminders in each to sync with the other.
@schmonz schmonz force-pushed the schmonz-git2.32.0-gitignore-symlink-switch branch from bdcdd92 to ae0efe1 Compare August 9, 2021 18:25
Copy link
Member

@DerDakon DerDakon left a comment

Choose a reason for hiding this comment

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

Good enough for 1.09, let's do it much better afterwards.

Copy link

@jlmuir jlmuir left a comment

Choose a reason for hiding this comment

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

LGTM.

@schmonz schmonz merged commit 8ea74d6 into master Aug 9, 2021
@schmonz schmonz deleted the schmonz-git2.32.0-gitignore-symlink-switch branch August 9, 2021 22:21
@DerDakon DerDakon mentioned this pull request Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants