Reserve a few more bits in the diff flags word to be used for future
whitespace rules. Add WS_INCOMPLETE_LINE without implementing the
behaviour (yet).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A patch file represents the incomplete line at the end of the file
with two lines, one that is the usual "context" with " " as the
first letter, "added" with "+" as the first letter, or "removed"
with "-" as the first letter that shows the content of the line,
plus an extra "\ No newline at the end of file" line that comes
immediately after it.
Ever since the apply machinery was written, the "git apply"
machinery parses "\ No newline at the end of file" line
independently, without even knowing what line the incomplete-ness
applies to, simply because it does not even remember what the
previous line was.
This poses a problem if we want to check and warn on an incomplete
line. Revamp the code that parses a fragment, to actually drop the
'\n' at the end of the incoming patch file that terminates a line,
so that check_whitespace() calls made from the code path actually
sees an incomplete as incomplete.
Note that the result of this parsing is not directly used by the
code path that applies the patch. apply_one_fragment() function
already checks if each of the patch text it handles is followed by a
line that begins with a backslash to drop the newline at the end of
the current line it is looking at. In a sense, this patch harmonizes
the behaviour of the parsing side to what is already done in the
application side.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff_symbol based output framework uses one DIFF_SYMBOL_* enum
value per the kind of output lines of "git diff", which corresponds
to one output line from the xdiff machinery used internally. Most
notably, DIFF_SYMBOL_PLUS and DIFF_SYMBOL_MINUS that correspond to
"+" and "-" lines are designed to always take a complete line, even
if the output from xdiff machinery may produce "\ No newline at the
end of file" immediately after them.
But this is not true in the rewrite-diff codepath, which completely
bypasses the xdiff machinery. Since the code path feeds the bytes
directly from the payload to the output routines, the output layer
has to deal with an incomplete line with DIFF_SYMBOL_PLUS and
DIFF_SYMBOL_MINUS, which never would see an incomplete line in the
normal code paths. This lack of final newline is compensated by an
ugly hack for a fabricated DIFF_SYMBOL_NO_LF_EOF token to inject an
extra newline to the output to simulate output coming from the xdiff
machinery.
Revamp the way the complete-rewrite code path feeds the lines to the
output layer by treating the last line of the pre/post image when it
is an incomplete line specially.
This lets us remove the DIFF_SYMBOL_NO_LF_EOF hack and use the usual
DIFF_SYMBOL_CONTEXT_INCOMPLETE code path, which will later learn how
to handle whitespace errors.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Everybody else, except for emit_rewrite_lines(), calls the
emit_callback data ecbdata. Make sure we call the same thing by
the same name for consistency.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create a helper function that reacts to "\ No newline at the end of
file" in preparation for unifying the incomplete line handling in
the code path that handles xdiff output and the code path that
bypasses xdiff and produces a complete-rewrite patch.
Currently the output from the DIFF_SYMBOL_CONTEXT_INCOMPLETE case
still (ab)uses the same code as what is used for context lines, but
that would change in a later step where we introduce support to treat
an incomplete line as a whitespace error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "\ No newline at the end of the file" can come after any of the
"-" (deleted preimage line), " " (unchanged line), or "+" (added
postimage line). In later steps in this series, we will start
treating a change that makes a file to end in an incomplete line
as a whitespace error, and we would need to know what the previous
line was when we react to "\ No newline" in the diff output. If
the previous line was a context (i.e., unchanged) line, the file
lacked the final newline before the change, and the change did not
touch that line and left it still incomplete, so we do not want to
warn in such a case.
Teach fn_out_consume() function to keep track of what the previous
line was, and prepare an otherwise empty switch statement to let us
react differently to "\ No newline" based on that.
Note that there is an existing curiosity (read: likely to be a bug)
in the code that increments line number in the preimage file every
time it sees a line with "\ No newline" on it, regardless of what
the previous line was. I left it as-is, because it does not affect
the main theme of this series, and more importantly, I do not think
it matters, as these numbers are used only to compare them with
blank_at_eof_in_{pre,post}image to issue a warning when we see more
empty line was added at the end, but by definition, after we see
"\ No newline at the end of the file" for an added line, we will not
see an added line for the file.
An independent audit to ensure that this curious increment can be
safely removed would make a good #leftoverbits clean-up (we may even
find some code that decrements this counter or over-increments the
other quantity this counter is compared with that compensates the
effect of this curious increment that hides a bug, in which case we
may also need to remove them).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The suppress-blank-empty feature abused the CONTEXT_INCOMPLETE
symbol that was meant to be used only for "\ No newline at the end
of file" code path.
The intent of the feature was to turn a context line we receive from
xdiff machinery (which always uses ' ' for context lines, even an
empty one) and spit it out as a truly empty line.
Perform such a conversion very locally at where a line from xdiff
that begins with ' ' is handled for output; there are many checks
before the control reaches such place that checks the first letter
of the diff output line to see if it is a context line, and having
to check for '\n' and treat it as a special case is error prone.
In order to catch similar hacks in the future, make sure the code
path that is meant for "\ No newline" case checks the first byte is
indeed a backslash.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the simple rule: if you need {} in one arm of the if/else
if/else... cascade, have {} in all of them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A comment in diff.c claimed that bits up to 12th (counting from 0th)
are whitespace rules, and 13th thru 15th are for new/old/context,
but it turns out it was miscounting. Correct them, and clarify
where the whitespace rule bits come from in the comment. Extend bit
assignment comments to cover bits used for color-moved, which
weren't described.
Also update the way these bit constants are defined to use (1 << N)
notation, instead of octal constants, as it tends to make it easier
to notice a breakage like this.
Sprinkle a few blank lines between logically distinct groups of CPP
macro definitions to make them easier to read.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
CI update.
* js/ci-github-actions-update:
build(deps): bump actions/github-script from 7 to 8
build(deps): bump actions/setup-python from 5 to 6
build(deps): bump actions/checkout from 4 to 5
build(deps): bump actions/download-artifact from 4 to 5
Recent OpenSSH creates the Unix domain socket to communicate with
ssh-agent under $HOME instead of /tmp, which causes our test to
fail doe to overly long pathname in our test environment, which has
been worked around by using "ssh-agent -T".
* ps/t7528-ssh-agent-uds-workaround:
t7528: work around ETOOMANY in OpenSSH 10.1 and newer
The "--short" option of "git status" that meant output for humans
and "-z" option to show NUL delimited output format did not mix
well, and colored some but not all things. The command has been
updated to color all elements consistently in such a case.
* jk/status-z-short-fix:
status: make coloring of "-z --short" consistent
An earlier addition to "git diff --no-index A B" to limit the
output with pathspec after the two directories misbehaved when
these directories were given with a trailing slash, which has been
corrected.
* jk/diff-no-index-with-pathspec-fix:
diff --no-index: fix logic for paths ending in '/'
Windows "real-time monitoring" interferes with the execution of
tests and affects negatively in both correctness and performance,
which has been disabled in Gitlab CI.
* ps/gitlab-ci-disable-windows-monitoring:
gitlab-ci: disable realtime monitoring to unbreak Windows jobs
The code to squelch output from "git diff -w --name-status"
etc. for paths that "git diff -w -p" would have stayed silent
leaked output from dry-run patch generation, which has been
corrected.
* jc/diff-from-contents-fix:
diff: make sure the other caller of diff_flush_patch_quietly() is silent
Recently we attempted to improve "git diff -w" and friends to
handle cases where patch output would be suppressed, but it
introduced a bug that emits unnecessary output, which has been
corrected.
* jk/diff-from-contents-fix:
diff: restore redirection to /dev/null for diff_from_contents
Recent OpenSSH creates the Unix domain socket to communicate with
ssh-agent under $HOME instead of /tmp, which causes our test to
fail doe to overly long pathname in our test environment, which has
been worked around by using "ssh-agent -T".
* ps/t7528-ssh-agent-uds-workaround:
t7528: work around ETOOMANY in OpenSSH 10.1 and newer
Build procedure for a few credential helpers (in contrib/) have
been updated.
* tu/credential-makefile-updates:
contrib/credential: harmonize Makefiles
The "--short" option of "git status" that meant output for humans
and "-z" option to show NUL delimited output format did not mix
well, and colored some but not all things. The command has been
updated to color all elements consistently in such a case.
* jk/status-z-short-fix:
status: make coloring of "-z --short" consistent
The code to squelch output from "git diff -w --name-status"
etc. for paths that "git diff -w -p" would have stayed silent
leaked output from dry-run patch generation, which has been
corrected.
* jc/diff-from-contents-fix:
diff: make sure the other caller of diff_flush_patch_quietly() is silent
Recently we attempted to improve "git diff -w" and friends to
handle cases where patch output would be suppressed, but it
introduced a bug that emits unnecessary output, which has been
corrected.
* jk/diff-from-contents-fix:
diff: restore redirection to /dev/null for diff_from_contents
In t7528 we spawn an SSH agent to verify that we can sign a commit via
it. This test has started to fail on some machines:
+++ ssh-agent
unix_listener_tmp: path "/home/pks/Development/git/build/test-output/trash directory.t7528-signed-commit-ssh/.ssh/agent/s.UTulegefEg.agent.UrPHumMXPq" too long for Unix domain socket
main: Couldn't prepare agent socket
As it turns out this is caused by a change in OpenSSH 10.1 [1]:
* ssh-agent(1), sshd(8): move agent listener sockets from /tmp to
under ~/.ssh/agent for both ssh-agent(1) and forwarded sockets
in sshd(8).
Instead of creating the socket in "/tmp", OpenSSH now creates the socket
in our home directory. And as the home directory gets modified to be
located in our test output directory we end up with paths that are
somewhat long. But Linux has a rather short limit of 108 characters for
socket paths, and other systems have even lower limits, so it is very
easy now to exceed the limit and run into the above error.
Work around the issue by using `ssh-agent -T`, which instructs it to
use the old behaviour and create the socket in "/tmp" again. This switch
has only been introduced with 10.1 though, so for older versions we have
to fall back to not using it. That's fine though, as older versions know
to put the socket into "/tmp" already.
An alternative approach would be to abbreviate the socket name itself so
that we create it as e.g. "sshsock" in the trash directory. But taking
the above example we'd still end up with a path that is 91 characters
long. So we wouldn't really have a lot of headroom, and it is quite
likely that some developers would see the issue on their machines.
[1]: https://www.openssh.com/txt/release-10.1
Reported-by: Xi Ruoyao <xry111@xry111.site>
Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Lauri Tirkkonen <lauri@hacktheplanet.fi>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Earlier, we added is a protection for the loop that computes "git
diff --quiet -w" to ensure calls to the diff_flush_patch_quietly()
helper stays quiet. Do the same for another loop that deals with
options like "--name-status" to make calls to the same helper.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation updates.
* je/doc-pull:
doc: git-pull: clarify how to exit a conflicted merge
doc: git-pull: delete the example
doc: git-pull: clarify options for integrating remote branch
doc: git-pull: move <repository> and <refspec> params
The beginning of SHA1-SHA256 interoperability work.
* bc/sha1-256-interop-01:
t1010: use BROKEN_OBJECTS prerequisite
t: allow specifying compatibility hash
fsck: consider gpgsig headers expected in tags
rev-parse: allow printing compatibility hash
docs: add documentation for loose objects
docs: improve ambiguous areas of pack format documentation
docs: reflect actual double signature for tags
docs: update offset order for pack index v3
docs: update pack index v3 format
CI update.
* js/ci-github-actions-update:
build(deps): bump actions/github-script from 7 to 8
build(deps): bump actions/setup-python from 5 to 6
build(deps): bump actions/checkout from 4 to 5
build(deps): bump actions/download-artifact from 4 to 5
Show option P in the prompt and explain it properly on a dedicated line
in online help and documentation.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update these Makefiles to be in line with other Makefiles from contrib
such as for contacts or subtree by making the following changes:
* Make the default settings after including config.mak.autogen and
config.mak.
* Add the missing $(CPPFLAGS) to the compiler command as well as the
missing $(CFLAGS) to the linker command.
* Use a pattern rule for compilation instead of a dedicated rule for
each compile unit.
* Get rid of $(MAIN), $(SRCS) and $(OBJS) and simply use their values
such as git-credential-libsecret and git-credential-libsecret.o.
* Strip @ from $(RM) to let the clean target rule be verbose.
* Define .PHONY for all special targets (all, clean).
Signed-off-by: Thomas Uhle <thomas.uhle@mailbox.tu-dresden.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>