So, both git diff and git show display "\ No newline at end of file" for symlinks.
I think this is related to how Git renders diffs, IMO it shouldn't display a warning about newlines for symlinks.
Am I making any sense here?
threads / discuss / 58616
About git reporting missing newline for symlinks
Subject: About git reporting missing newline for symlinks
Junio C HamanoRe: About git reporting missing newline for symlinks
Ignacio Taranto <ignacio.taranto@eclypsium.com> writes:
Show 7 quoted lines
> So, both git diff and git show display "\ No newline at end of file" > for symlinks. > > I think this is related to how Git renders diffs, IMO it shouldn't > display a warning about newlines for symlinks. > > Am I making any sense here?
Yes, but not really.
It is not "warning" at all. The users want to know when compared contents do or do not end with an incomplete line at the end, and the "\ No newline" is the diff's way to give that single bit of information to us.
And the contents of a symbolic link typically is a single incomplete line, so it is expected to see "\ No newline" when comparing them.
This is important as "git diff | git -C ../some/where/else apply" should be able to recreate the change in the current repository (which may have change to or addition of a symbolic link) in the other repository, and "git apply" on the receiving end must be able to tell that the symbolic link it needs to create in the other repository must not have an extra LF at the end.
Re: About git reporting missing newline for symlinks
That was very well explained, thank you.
The problem seems to be that some people interpret this message as a real warning since it's often a good practice to always end text files with a newline (at least on *nix systems). Of course this doesn't apply to symlinks at all.
On Wed, Oct 12, 2022 at 6:31 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 28 quoted lines
> > Ignacio Taranto <ignacio.taranto@eclypsium.com> writes: > > > So, both git diff and git show display "\ No newline at end of file" > > for symlinks. > > > > I think this is related to how Git renders diffs, IMO it shouldn't > > display a warning about newlines for symlinks. > > > > Am I making any sense here? > > Yes, but not really. > > It is not "warning" at all. The users want to know when compared > contents do or do not end with an incomplete line at the end, and > the "\ No newline" is the diff's way to give that single bit of > information to us. > > And the contents of a symbolic link typically is a single incomplete > line, so it is expected to see "\ No newline" when comparing them. > > This is important as "git diff | git -C ../some/where/else apply" > should be able to recreate the change in the current repository > (which may have change to or addition of a symbolic link) in the > other repository, and "git apply" on the receiving end must be able > to tell that the symbolic link it needs to create in the other > repository must not have an extra LF at the end. >
Ævar Arnfjörð BjarmasonRe: About git reporting missing newline for symlinks
On Wed, Oct 12 2022, Junio C Hamano wrote:
Show 26 quoted lines
> Ignacio Taranto <ignacio.taranto@eclypsium.com> writes: > >> So, both git diff and git show display "\ No newline at end of file" >> for symlinks. >> >> I think this is related to how Git renders diffs, IMO it shouldn't >> display a warning about newlines for symlinks. >> >> Am I making any sense here? > > Yes, but not really. > > It is not "warning" at all. The users want to know when compared > contents do or do not end with an incomplete line at the end, and > the "\ No newline" is the diff's way to give that single bit of > information to us. > > And the contents of a symbolic link typically is a single incomplete > line, so it is expected to see "\ No newline" when comparing them. > > This is important as "git diff | git -C ../some/where/else apply" > should be able to recreate the change in the current repository > (which may have change to or addition of a symbolic link) in the > other repository, and "git apply" on the receiving end must be able > to tell that the symbolic link it needs to create in the other > repository must not have an extra LF at the end.
In apply.c's parse_fragment() we seem to only care that we find a "\"-line that's at least the length of "\ No newline...".
I wonder what (if any) compatibility issues we'd have if we emitted e.g.:
\ The filename pointed to by the symlink does not end in a newline
Junio C HamanoRe: About git reporting missing newline for symlinks
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> In apply.c's parse_fragment() we seem to only care that we find a > "\"-line that's at least the length of "\ No newline...".
If I recall correctly, this was done very much on purpose back in 2005 on top of what I and Linus did originally (which was specific to "No newline" written in C locale). Because the input from "git apply" may not necessarily be the output from "git diff", and in the older days, "git diff" spawned platform "diff" to show diff, it would have been subject to l10n.
With GNU diff, I can do
$ echo -n one >/var/tmp/1; echo -n two >/var/tmp/2
$ LC_ALL=ja_JP.utf8 diff -u /var/tmp/[12]
--- /var/tmp/1 2022-10-13 08:40:55.393209930 -0700
+++ /var/tmp/2 2022-10-13 08:40:59.269538397 -0700
@@ -1 +1 @@
-one
\ ファイル末尾に改行がありません
+two
\ ファイル末尾に改行がありませんAs it does not consider a file that ends with an incomplete line a text file [*], I do not think POSIX says anything about lines that begin with "\" in the "diff" output, but I would be surprised if GNU patch (or anybody else's, for that matter) did not ignore text on the line, because all of them must be able to parse the above output.
> I wonder what (if any) compatibility issues we'd have if we emitted > e.g.: > > \ The filename pointed to by the symlink does not end in a newline
While I do not think it would break anybody, I doubt it would give us much value. One line above that output is a line that any user, who is vaguely familiar with the contents being compared, can recognize as giving a pathname, the contents of the symbolic link.
[References]
https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xbd_chap03.html#tag_21_03_00_83
Ævar Arnfjörð BjarmasonRe: About git reporting missing newline for symlinks
On Thu, Oct 13 2022, Junio C Hamano wrote:
Show 30 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> In apply.c's parse_fragment() we seem to only care that we find a >> "\"-line that's at least the length of "\ No newline...". > > If I recall correctly, this was done very much on purpose back in > 2005 on top of what I and Linus did originally (which was specific > to "No newline" written in C locale). Because the input from "git > apply" may not necessarily be the output from "git diff", and in the > older days, "git diff" spawned platform "diff" to show diff, it would > have been subject to l10n. > > With GNU diff, I can do > > $ echo -n one >/var/tmp/1; echo -n two >/var/tmp/2 > $ LC_ALL=ja_JP.utf8 diff -u /var/tmp/[12] > --- /var/tmp/1 2022-10-13 08:40:55.393209930 -0700 > +++ /var/tmp/2 2022-10-13 08:40:59.269538397 -0700 > @@ -1 +1 @@ > -one > \ ファイル末尾に改行がありません > +two > \ ファイル末尾に改行がありません > > As it does not consider a file that ends with an incomplete line a > text file [*], I do not think POSIX says anything about lines that > begin with "\" in the "diff" output, but I would be surprised if GNU > patch (or anybody else's, for that matter) did not ignore text on > the line, because all of them must be able to parse the above > output.
That's really informative, thanks.
Show 9 quoted lines
>> I wonder what (if any) compatibility issues we'd have if we emitted >> e.g.: >> >> \ The filename pointed to by the symlink does not end in a newline > > While I do not think it would break anybody, I doubt it would give > us much value. One line above that output is a line that any user, > who is vaguely familiar with the contents being compared, can > recognize as giving a pathname, the contents of the symbolic link.
Clearly it confused the initial reporter upthread :) I'm not going to pursue it, but it seems to me that we're being sloppy here in including a message that makes more sense for a normal diff when we could consider the mode(s) involved, as it's not abnormal for a filename to be missing a trailing "\n" (having a "\n" being the odd case).
Anyway, I'm not going to pursue changing the message, but maybe if someone else is interested the above compatibility notes would be valuable, thanks!
Junio C HamanoRe: About git reporting missing newline for symlinks
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
>>> \ The filename pointed to by the symlink does not end in a newline >> >> While I do not think it would break anybody, I doubt it would give >> us much value. One line above that output is a line that any user, >> who is vaguely familiar with the contents being compared, can >> recognize as giving a pathname, the contents of the symbolic link. > > Clearly it confused the initial reporter upthread :)
But to such a user, I highly suspect that the rephased message above still looks like a warning, and will result in the same reaction.
IOW, you want to explain why "does not end in a newline" is worth expressing in the output. Saying "does not end in a newline" alone would tell the user what they already know (i.e. the symlink stores the target filename without an extra LF at the end).
Ævar Arnfjörð BjarmasonRe: About git reporting missing newline for symlinks
On Thu, Oct 13 2022, Junio C Hamano wrote:
Show 18 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >>>> \ The filename pointed to by the symlink does not end in a newline >>> >>> While I do not think it would break anybody, I doubt it would give >>> us much value. One line above that output is a line that any user, >>> who is vaguely familiar with the contents being compared, can >>> recognize as giving a pathname, the contents of the symbolic link. >> >> Clearly it confused the initial reporter upthread :) > > But to such a user, I highly suspect that the rephased message above > still looks like a warning, and will result in the same reaction. > > IOW, you want to explain why "does not end in a newline" is worth > expressing in the output. Saying "does not end in a newline" alone > would tell the user what they already know (i.e. the symlink stores > the target filename without an extra LF at the end).
Yes, but isn't the point of the report/confusion that we're inserting what looks like the warning you get when you forget a \n at the end of a source file, so a user might wonder why they're seeing it at all.
Whereas what we're *really* doing there is not really about that at all, but just inserting a bit of magic so that the diff format & its consumers grok that this line we're inserting there isn't supposed to have a \n, as we're working with a filename.
Maybe e.g.: diff --git a/RelNotes b/RelNotes index d505db645be..758368388a4 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.38.0.txt \ No newline at end of file +Documentation/RelNotes/2.39.0.txt \ No newline at end of file
Would, for those users, be less confusing as:
diff --git a/RelNotes b/RelNotes index d505db645be..758368388a4 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.38.0.txt \ The symlink above has no trailing NL in its filename +Documentation/RelNotes/2.39.0.txt \ The symlink above has no trailing NL in its filename
*dunno* :)
Junio C HamanoRe: About git reporting missing newline for symlinks
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 13 quoted lines
> Would, for those users, be less confusing as: > > diff --git a/RelNotes b/RelNotes > index d505db645be..758368388a4 120000 > --- a/RelNotes > +++ b/RelNotes > @@ -1 +1 @@ > -Documentation/RelNotes/2.38.0.txt > \ The symlink above has no trailing NL in its filename > +Documentation/RelNotes/2.39.0.txt > \ The symlink above has no trailing NL in its filename > > *dunno* :)
I do not think so, but "The symlink lacks the terminating LF and it is perfectly normal" would be an OK thing to say. It would make it clear that it is not a warning message.
Philip OakleyRe: About git reporting missing newline for symlinks
On 13/10/2022 20:33, Ævar Arnfjörð Bjarmason wrote:
Show 52 quoted lines
> On Thu, Oct 13 2022, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>>>> \ The filename pointed to by the symlink does not end in a newline >>>> While I do not think it would break anybody, I doubt it would give >>>> us much value. One line above that output is a line that any user, >>>> who is vaguely familiar with the contents being compared, can >>>> recognize as giving a pathname, the contents of the symbolic link. >>> Clearly it confused the initial reporter upthread :) >> But to such a user, I highly suspect that the rephased message above >> still looks like a warning, and will result in the same reaction. >> >> IOW, you want to explain why "does not end in a newline" is worth >> expressing in the output. Saying "does not end in a newline" alone >> would tell the user what they already know (i.e. the symlink stores >> the target filename without an extra LF at the end). > Yes, but isn't the point of the report/confusion that we're inserting > what looks like the warning you get when you forget a \n at the end of a > source file, so a user might wonder why they're seeing it at all. > > Whereas what we're *really* doing there is not really about that at all, > but just inserting a bit of magic so that the diff format & its > consumers grok that this line we're inserting there isn't supposed to > have a \n, as we're working with a filename. > > Maybe e.g.: > > diff --git a/RelNotes b/RelNotes > index d505db645be..758368388a4 120000 > --- a/RelNotes > +++ b/RelNotes > @@ -1 +1 @@ > -Documentation/RelNotes/2.38.0.txt > \ No newline at end of file > +Documentation/RelNotes/2.39.0.txt > \ No newline at end of file > > Would, for those users, be less confusing as: > > diff --git a/RelNotes b/RelNotes > index d505db645be..758368388a4 120000 > --- a/RelNotes > +++ b/RelNotes > @@ -1 +1 @@ > -Documentation/RelNotes/2.38.0.txt > \ The symlink above has no trailing NL in its filename > +Documentation/RelNotes/2.39.0.txt > \ The symlink above has no trailing NL in its filename > > *dunno* :) >
Pondering on why it is thought of as a warning to be treated as an error, maybe the message implies something is missing via the negative "No" assertion, rather than simply being informative [1].
Perhaps swapping out the "No" and instead using something like "\ Without a newline at end of file" would better convey that it's just information about the file's format, and in no way a real problem. Just a thought. -- Philip [1] My comment in Git Users discussion: https://groups.google.com/d/msgid/git-users/c4f53bb2-cff6-4a76-8fa7-cd34ca88ce63n%40googlegroups.com <https://groups.google.com/d/msgid/git-users/c4f53bb2-cff6-4a76-8fa7-cd34ca88ce63n%40googlegroups.com?utm_medium=email&utm_source=footer>
Re: About git reporting missing newline for symlinks
On Thu, Oct 13, 2022 at 10:55 PM Philip Oakley <philipoakley@iee.email> wrote:
> > Pondering on why it is thought of as a warning to be treated as an > error, maybe the message implies something is missing via the negative > "No" assertion, rather than simply being informative [1].
Very likely. I imagine most users use diff to view the changes they made without knowledge of the use of diffs in git-apply. Without that background it's more likely to be interpreted as an opinionated comment as opposed to being informative.
> Perhaps swapping out the "No" and instead using something like "\ > Without a newline at end of file" would better convey that it's just > information about the file's format, and in no way a real problem. Just > a thought.
That's probably less likely to be read as a warning
brian m. carlsonRe: About git reporting missing newline for symlinks
On 2022-10-12 at 21:17:04, Ignacio Taranto wrote:
Show 8 quoted lines
> So, both git diff and git show display "\ No newline at end of file" > for symlinks. > > I think this is related to how Git renders diffs, IMO it shouldn't > display a warning about > newlines for symlinks. > > Am I making any sense here?
What Git shows for a symlink is the destination of the symlink. On Unix systems other than macOS, path names may contain any byte other than NUL, including a newline.
While it obviously is not a good practice to include newlines in your paths, it is possible, and Git would not be able to round-trip symlinks where the final character of the destination is a newline if we didn't handle trailing newlines properly. As a result, we're not likely to change things here because it's required for correctness.
-- brian m. carlson (he/him or they/them) Toronto, Ontario, CA
Junio C HamanoRe: About git reporting missing newline for symlinks
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 19 quoted lines
> On 2022-10-12 at 21:17:04, Ignacio Taranto wrote: >> So, both git diff and git show display "\ No newline at end of file" >> for symlinks. >> >> I think this is related to how Git renders diffs, IMO it shouldn't >> display a warning about >> newlines for symlinks. >> >> Am I making any sense here? > > What Git shows for a symlink is the destination of the symlink. On > Unix systems other than macOS, path names may contain any byte other > than NUL, including a newline. > > While it obviously is not a good practice to include newlines in your > paths, it is possible, and Git would not be able to round-trip symlinks > where the final character of the destination is a newline if we didn't > handle trailing newlines properly. As a result, we're not likely to > change things here because it's required for correctness.
Just a fun thought experiment, but I wonder what would happen if a user chooses to add .clean and .smudge filter that adds and strips a LF at the end ;-)
brian m. carlsonRe: About git reporting missing newline for symlinks
On 2022-10-12 at 21:34:55, Junio C Hamano wrote:
> Just a fun thought experiment, but I wonder what would happen if a > user chooses to add .clean and .smudge filter that adds and strips > a LF at the end ;-)
I don't believe those are invoked on symlinks, since I don't recall us having to handle symlinks in Git LFS, which works using a smudge and clean filter.
-- brian m. carlson (he/him or they/them) Toronto, Ontario, CA
Taylor BlauRe: About git reporting missing newline for symlinks
On Wed, Oct 12, 2022 at 09:42:12PM +0000, brian m. carlson wrote:
Show 8 quoted lines
> On 2022-10-12 at 21:34:55, Junio C Hamano wrote: > > Just a fun thought experiment, but I wonder what would happen if a > > user chooses to add .clean and .smudge filter that adds and strips > > a LF at the end ;-) > > I don't believe those are invoked on symlinks, since I don't recall us > having to handle symlinks in Git LFS, which works using a smudge and > clean filter.
That is right. It matches my memory, at least, though it has been a very long time since I've thought that hard about the clean / smudge code.
Thanks, Taylor