This is done to
(1) ensure MERGE_HEAD is a ref,
(2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()
(3) error out when MERGE_HEAD is a symref.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some functions in Git's source code follow the convention that returning
a negative value indicates a fatal error, e.g. repository corruption.
Let's use this convention in `repo_in_merge_bases()` to report when one
of the specified commits is missing (i.e. when `repo_parse_commit()`
reports an error).
Also adjust the callers of `repo_in_merge_bases()` to handle such
negative return values.
Note: As of this patch, errors are returned only if any of the specified
merge heads is missing. Over the course of the next patches, missing
commits will also be reported by the `paint_down_to_common()` function,
which is called by `repo_in_merge_bases_many()`, and those errors will
be properly propagated back to the caller at that stage.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently this function treats unrelated commit histories the same way
as commit histories with missing commit objects.
Typically, missing commit objects constitute a corrupt repository,
though, and should be reported as such. The next commits will make it
so, but there is one exception: In `git fetch --update-shallow` we
_expect_ commit objects to be missing, and we do want to treat the
now-incomplete commit histories as unrelated.
To allow for that, let's introduce an additional parameter that is
passed to `repo_in_merge_bases_many()` to trigger this behavior, and use
it in the two callers in `shallow.c`.
This commit changes behavior slightly: unless called from the
`shallow.c` functions that set the `ignore_missing_commits` bit, any
non-existing tip commit that is passed to `repo_in_merge_bases_many()`
will now result in an error.
Note: When encountering missing commits while traversing the commit
history in search for merge bases, with this commit there won't be a
change in behavior just yet, their children will still be interpreted as
root commits. This bug will get fixed by follow-up commits.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a commit is missing, we return early (currently pretending that no
merge basis could be found in that case). At that stage, it is possible
that a merge base could have been found already, and added to the
`result`, which is now leaked.
The priority queue has a similar issue: There might still be a commit in
that queue.
Let's release both, to address the potential memory leaks.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 9830926c7d (rev-list: add commit object support in `--missing`
option, 2023-10-27) we fixed the `--missing` option in `git rev-list`
so that it works with missing commits, not just blobs/trees.
Unfortunately, such a command was still failing with a "fatal: bad
object <oid>" if it was passed a missing commit, blob or tree as an
argument (before the rev walking even begins). This was fixed in a
recent commit.
That fix still doesn't work when an argument passed to the command is
an annotated tag pointing to a missing commit though. In that case
`git rev-list --missing=...` still errors out with a "fatal: bad
object <oid>" error where <oid> is the object ID of the missing
commit.
Let's fix this issue, and also, while at it, let's add tests not just
for annotated tags but also for regular tags and branches.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Credential helper based on libsecret (in contrib/) has been updated
to handle an empty password correctly.
* mh/libsecret-empty-password-fix:
libsecret: retrieve empty password
Some parts of command line completion script (in contrib/) have
been micro-optimized.
* bb/completion-no-grep-into-awk:
completion: use awk for filtering the config entries
Variants of vimdiff learned to honor mergetool.<variant>.layout settings.
* km/mergetool-vimdiff-layout-fallback:
mergetools: vimdiff: use correct tool's name when reading mergetool config
"git tag --column" failed to check the exit status of its "git
column" invocation, which has been corrected.
* rj/tag-column-fix:
tag: error when git-column fails
"git am --help" now tells readers what actions are available in
"git am --whitespace=<action>", in addition to saying that the
option is passed through to the underlying "git apply".
* jc/am-whitespace-doc:
doc: add shortcut to "am --whitespace=<action>"
Under normal circumstances, it shouldn't ever happen that a repository
has no HEAD reference. In fact, git-update-ref(1) would fail any request
to delete the HEAD reference, and a newly initialized repository always
pre-creates it, too.
We have however changed git-clone(1) to partially initialize the
refdb just up to the point where remote helpers can find the
repository. With that change, we are going to run into a situation
where repositories have no refs at all.
Now there is a very particular edge case in this situation: when
preparing an empty ref transacton, we end up returning whatever value
`read_ref_without_reload()` returned to the caller. Under normal
conditions this would be fine: "HEAD" should usually exist, and thus the
function would return `0`. But if "HEAD" doesn't exist, the function
returns a positive value which we end up returning to the caller.
Fix this bug by resetting the return code to `0` and add a test.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use __git to invoke "worktree list" in __git_complete_worktree_paths, to
respect any "-C" and "--git-dir" options present on the command line.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 18c9cb7524 (builtin/clone: create the refdb with the correct object
format, 2023-12-12), we have changed git-clone(1) so that it delays
creation of the refdb until after it has learned about the remote's
object format. This change was required for the reftable backend, which
encodes the object format into the tables. So if we pre-initialized the
refdb with the default object format, but the remote uses a different
object format than that, then the resulting tables would have encoded
the wrong object format.
This change unfortunately breaks remote helpers which try to access the
repository that is about to be created. Because the refdb has not yet
been initialized at the point where we spawn the remote helper, we also
don't yet have "HEAD" or "refs/". Consequently, any Git commands ran by
the remote helper which try to access the repository would fail because
it cannot be discovered.
This is essentially a chicken-and-egg problem: we cannot initialize the
refdb because we don't know about the object format. But we cannot learn
about the object format because the remote helper may be unable to
access the partially-initialized repository.
Ideally, we would address this issue via capabilities. But the remote
helper protocol is not structured in a way that guarantees that the
capability announcement happens before the remote helper tries to access
the repository.
Instead, fix this issue by partially initializing the refdb up to the
point where it becomes discoverable by Git commands.
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Setting this environment variable causes the sequencer to display a
custom message when it stops for the user to resolve conflicts and
remove CHERRY_PICK_HEAD. Setting it in "git rebase" is a vestige of
the scripted implementation, now that it is a builtin command we do
not need to communicate with the sequencer machinery via environment
variables.
Move the conflicts advice to use when rebasing into
sequencer.c so we do not need to pass it via the environment.
Note that we retain the changes in e4301f73ff (sequencer: unset
GIT_CHERRY_PICK_HELP for 'exec' commands, 2024-02-02) just in case
GIT_CHERRY_PICK_HELP is set in the environment when "git rebase" is
run.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Modeling after how the `--no-replace-objects` option is made usable
across subprocess spawning (e.g., cURL based remote helpers are
spawned as a separate process while running "git fetch"), allow the
`--no-lazy-fetch` option to be passed across process boundaries.
Do not model how the value of GIT_NO_REPLACE_OBJECTS environment
variable is ignored, though. Just use the usual git_env_bool() to
allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH"
to be equivalents.
Also do not model how the request is not propagated to subprocesses
we spawn (e.g. "git clone --local" that spawns a new process to work
in the origin repository, while the original one working in the
newly created one) by the "--no-replace-objects" option, as this "do
not lazily fetch from the promisor" is more about a per-request
debugging aid, not "this repository's promisor should not be relied
upon" property specific to a repository.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
prepare_to_commit has some logic to figure out whether merge already
added a scissors line, and therefore it shouldn't add another. Now that
wt_status_add_cut_line has built-in state for whether it has
already added a previous line, just set that state instead, and then
remove that condition from subsequent calls to wt_status_add_cut_line.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git commit --cleanup=scissors -v` prints two scissors lines:
one at the start of the comment lines, and the other right before the
diff. This is redundant, and pushes the diff further down in the user's
editor than it needs to be.
Make wt_status_add_cut_line() remember if it has added a cut line before,
and avoid adding a redundant one.
Add a test for this.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The reason why we require the <git-compat-util.h> file to be the
first header file to be included is because it insulates other
header files and source files from platform differences, like which
system header files must be included in what order, and what C
preprocessor feature macros must be defined to trigger certain
features we want out of the system.
We tried to clarify the rule in the coding guidelines document, but
the wording was a bit fuzzy that can lead to misinterpretations like
you can include <xdiff/xinclude.h> only to avoid having to include
<git-compat-util.h> even if you have nothing to do with the xdiff
implementation, for example. "You do not have to include more than
one of these" was also misleading and would have been puzzling if
you _needed_ to depend on more than one of these approved headers
(answer: you are allowed to include them all if you need the
declarations in them for reasons other than that you want to avoid
including compat-util yourself).
Instead of using the phrase "approved headers", enumerate them as
exceptions, each labeled with its intended audiences, to avoid such
misinterpretations. The structure also makes it easier to add new
exceptions, so add the description of "t/unit-tests/test-lib.h"
being an exception only for the unit tests implementation as an
example.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Kyle Lippincott <spectral@google.com>
Acked-by: Elijah Newren <newren@gmail.com>
This is a minor follow-up to cb00f524df (rebase: rewrite
--(no-)autosquash documentation, 2023-11-14) to fix a typo introduced in
that commit.
Signed-off-by: Richard Macklin <code@rmacklin.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This variable is used as the primary way to disable the object
replacement mechanism, with the "--no-replace-objects" command line
option as an end-user visible way to set it, but has not been
documented.
The original reason why it was left undocumented might be because it
was meant as an internal implementation detail, but the thing is,
that our tests use the environment variable directly without the
command line option, and there certainly are folks who learned its
use from there, making it impossible to deprecate or change its
behaviour by now.
Add documentation and note that for this variable, unlike many
boolean-looking environment variables, only the presence matters,
not what value it is set to.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
More tests that are marked as "ref-files only" have been updated to
improve test coverage of reftable backend.
* ps/ref-tests-update-even-more:
t7003: ensure filter-branch prunes reflogs with the reftable backend
t2011: exercise D/F conflicts with HEAD with the reftable backend
t1405: remove unneeded cleanup step
t1404: make D/F conflict tests compatible with reftable backend
t1400: exercise reflog with gaps with reftable backend
t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
t: move tests exercising the "files" backend
Teach "git checkout -p" and friends that "@" is a synonym for
"HEAD".
* gt/at-is-synonym-for-head-in-add-patch:
add -p tests: remove PERL prerequisites
add-patch: classify '@' as a synonym for 'HEAD'
"git column" has been taught to reject negative padding value, as
it would lead to nonsense behaviour including division by zero.
* kh/column-reject-negative-padding:
column: guard against negative padding
column: disallow negative padding
Adjust use of "rev-list --missing" in an existing tests so that it
does not depend on a buggy failure mode.
* jc/t9210-lazy-fix:
t9210: do not rely on lazy fetching to fail
The code to iterate over refs with the reftable backend has seen
some optimization.
* ps/reftable-iteration-perf:
reftable/reader: add comments to `table_iter_next()`
reftable/record: don't try to reallocate ref record name
reftable/block: swap buffers instead of copying
reftable/pq: allocation-less comparison of entry keys
reftable/merged: skip comparison for records of the same subiter
reftable/merged: allocation-less dropping of shadowed records
reftable/record: introduce function to compare records by key
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
* cp/apply-core-filemode:
apply: code simplification
apply: correctly reverse patch's pre- and post-image mode bits
apply: ignore working tree filemode when !core.filemode
Integrate the reftable code into the refs framework as a backend.
* ps/reftable-backend:
refs/reftable: fix leak when copying reflog fails
ci: add jobs to test with the reftable backend
refs: introduce reftable backend
Refactor the code that handles refresh events for pathnames that do
not contain a trailing slash. Instead of using a custom loop to try
to scan the index and detect if the FSEvent named a file or might be a
directory prefix, use the recently created helper function to do that.
Also update the comments to describe what and why we are doing this.
On platforms that DO NOT annotate FS events with a trailing
slash, if we fail to find an exact match for the pathname
in the index, we do not know if the pathname represents a
directory or simply an untracked file. Pretend that the pathname
is a directory and try again before assuming it is an untracked
file.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach the refresh callback helper function for directory FSEvents to
return the number of cache-entries that were invalidated in response
to a directory event.
This will be used in a later commit to help determine if the observed
pathname in the FSEvent was a (possibly) case-incorrect directory
prefix (on a case-insensitive filesystem) of one or more actual
cache-entries.
If there exists at least one case-insensitive prefix match, then we
can assume that the directory is a (case-incorrect) prefix of at least
one tracked item rather than a completely unknown/untracked file or
directory.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the call to invalidate the untracked-cache for the FSEvent
pathname into the two helper functions.
In a later commit in this series, we will call these helpers
from other contexts and it safer to include the UC invalidation
in the helpers than to remember to also add it to each helper
call-site.
This has the side-effect of invalidating the UC *before* we
invalidate the ce_flags in the cache-entry. These activities
are independent and do not affect each other. Also, by doing
the UC work first, we can avoid worrying about "early returns"
or the need for the usual "goto the end" in each of the
handler functions.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update fsmonitor_refresh_callback() to use the new
untracked_cache_invalidate_trimmed_path() to invalidate
the cache using the observed pathname without needing to
modify the caller's buffer.
Previously, we modified the caller's buffer when the observed pathname
contained a trailing slash (and did not restore it). This wasn't a
problem for the single use-case caller, but felt dirty nontheless. In
a later commit we will want to invalidate case-corrected versions of
the pathname (using possibly borrowed pathnames from the name-hash or
dir-name-hash) and we may not want to keep the tradition of altering
the passed-in pathname.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Create a wrapper function for untracked_cache_invalidate_path()
that silently trims a trailing slash, if present, before calling
the wrapped function.
The untracked cache expects to be called with a pathname that
does not contain a trailing slash. This can make it inconvenient
for callers that have a directory path. Lets hide this complexity.
This will be used by a later commit in the FSMonitor code which
may receive directory pathnames from an FSEvent.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code that handles unqualified FSEvents (without a trailing
slash) into a helper function.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Improve documentation of the refresh callback helper function
used for directory FSEvents.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code to handle directory FSEvents (containing pathnames with
a trailing slash) into a helper function.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The FSMonitor client code trusts the spelling of the pathnames in the
FSEvents received from the FSMonitor daemon. On case-insensitive file
systems, these OBSERVED pathnames may be spelled differently than the
EXPECTED pathnames listed in the .git/index. This causes a miss when
using `index_name_pos()` which expects the given case to be correct.
When this happens, the FSMonitor client code does not update the state
of the CE_FSMONITOR_VALID bit when refreshing the index (and before
starting to scan the worktree).
This results in modified files NOT being reported by `git status` when
there is a discrepancy in the case-spelling of a tracked file's
pathname.
This commit contains a (rather contrived) test case to demonstrate
this. A later commit in this series will update the FSMonitor client
code to recognize these discrepancies and update the CE_ bit accordingly.
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
index_dir_exists() returns a boolean to indicate if there is a
case-insensitive match in the directory name-hash, but does not
provide the caller with the exact spelling of that match.
Create index_dir_find() to do the case-insensitive search *and*
optionally return the spelling of the matched directory prefix in a
provided strbuf.
To avoid code duplication, convert index_dir_exists() to be a trivial
wrapper around the new index_dir_find().
Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file has been added as part of 2232a88ab6 (attr: add builtin
objectmode values support, 2023-11-16) and most likely serves no
relevant purpose.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new advice type 'submoduleMergeConflict' for the error message
shown when a non-trivial submodule conflict is encountered, which
was added in 4057523a40 (submodule merge: update conflict error
message, 2022-08-04). That commit mentions making this message an
advice as possible future work. The message can now be disabled
with the advice mechanism.
Update the tests as the expected message now appears on stderr instead
of stdout.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit special-cased get_oid_basic()'s handling of ref@{n}
for a reflog with n entries. But its special case doesn't work for
ref@{0} in an empty reflog, because read_ref_at() dies when it notices
the empty reflog!
We can make this work by special-casing this in read_ref_at(). It's
somewhat gross, for two reasons:
1. We have no reflog entry to describe in the "msg" out-parameter. So
we have to leave it uninitialized or make something up.
2. Likewise, we have no oid to put in the "oid" out-parameter. Leaving
it untouched is actually the best thing here, as all of the callers
will have initialized it with the current ref value via
repo_dwim_log(). This is rather subtle, but it is how things worked
in 6436a20284 (refs: allow @{n} to work with n-sized reflog,
2021-01-07) before we reverted it.
The key difference from 6436a20284 here is that we'll return "1" to
indicate that we _didn't_ find the requested reflog entry. Coupled with
the special-casing in get_oid_basic() in the previous commit, that's
enough to make looking up ref@{0} work, and we can flip 6436a20284's
test back to expect_success.
It also means that the call in show-branch which segfaulted with
6436a20284 (and which is now tested in t3202) remains OK. The caller
notices that we could not find any reflog entry, and so it breaks out of
its loop, showing nothing. This is different from the current behavior
of producing an error, but it's just as reasonable (and is exactly what
we'd do if you asked it to walk starting at ref@{1} but there was only 1
entry).
Thus nobody should actually look at the reflog entry info we return. But
we'll still put in some fake values just to be on the safe side, since
this is such a subtle and confusing interface. Likewise, we'll document
what's going on in a comment above the function declaration. If this
were a function with a lot of callers, the footgun would probably not be
worth it. But it has only ever had two callers in its 18-year existence,
and it seems unlikely to grow more. So let's hold our noses and let
users enjoy the convenience of a simulated ref@{0}.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The goal of 6436a20284 (refs: allow @{n} to work with n-sized reflog,
2021-01-07) was that if we have "n" entries in a reflog, we should still
be able to resolve ref@{n} by looking at the "old" value of the oldest
entry.
Commit 6436a20284 tried to put the logic into read_ref_at() by shifting
its idea of "n" by one. But we reverted that in the previous commit,
since it led to bugs in other callers which cared about the details of
the reflog entry we found. Instead, let's put the special case into the
caller that resolves @{n}, as it cares only about the oid.
read_ref_at() is even kind enough to return the "old" value from the
final reflog; it just returns "1" to signal to us that we ran off the
end of the reflog. But we can notice in the caller that we read just
enough records for that "old" value to be the one we're looking for, and
use it.
Note that read_ref_at() could notice this case, too, and just return 0.
But we don't want to do that, because the caller must be made aware that
we only found the oid, not an actual reflog entry (and the call sites in
show-branch do care about this).
There is one complication, though. When read_ref_at() hits a truncated
reflog, it will return the "old" value of the oldest entry only if it is
not the null oid. Otherwise, it actually returns the "new" value from
that entry! This bit of fudging is due to d1a4489a56 (avoid null SHA1 in
oldest reflog, 2008-07-08), where asking for "ref@{20.years.ago}" for a
ref created recently will produce the initial value as a convenience
(even though technically it did not exist 20 years ago).
But this convenience is only useful for time-based cutoffs. For
count-based cutoffs, get_oid_basic() has always simply complained about
going too far back:
$ git rev-parse HEAD@{20}
fatal: log for 'HEAD' only has 16 entries
and we should continue to do so, rather than returning a nonsense value
(there's even a test in t1508 already which covers this). So let's have
the d1a4489a56 code kick in only when doing timestamp-based cutoffs.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 6436a20284.
The idea of that commit is that if read_ref_at() is counting back to the
Nth reflog but the reflog is short by one entry (e.g., because it was
pruned), we can find the oid of the missing entry by looking at the
"before" oid value of the entry that comes after it (whereas before, we
looked at the "after" value of each entry and complained that we
couldn't find the one from before the truncation).
This works fine for resolving the oid of ref@{n}, as it is used by
get_oid_basic(), which does not look at any other aspect of the reflog
we found (e.g., its timestamp or message). But there's another caller of
read_ref_at(): in show-branch we use it to walk over the reflog, and we
do care about the reflog entry. And so that commit broke "show-branch
--reflog"; it shows the reflog message for ref@{0} as ref@{1}, ref@{1}
as ref@{2}, and so on.
For example, in the new test in t3202 we produce:
! [branch@{0}] (0 seconds ago) commit: three
! [branch@{1}] (0 seconds ago) commit: three
! [branch@{2}] (60 seconds ago) commit: two
! [branch@{3}] (2 minutes ago) reset: moving to HEAD^
instead of the correct:
! [branch@{0}] (0 seconds ago) commit: three
! [branch@{1}] (60 seconds ago) commit: two
! [branch@{2}] (2 minutes ago) reset: moving to HEAD^
! [branch@{3}] (2 minutes ago) commit: one
But there's another bug, too: because it is looking at the "old" value
of the reflog after the one we're interested in, it has to special-case
ref@{0} (since there isn't anything after it). That's why it doesn't
show the offset bug in the output above. But this special-case code
fails to handle the situation where the reflog is empty or missing; it
returns success even though the reflog message out-parameter has been
left uninitialized. You can't trigger this through get_oid_basic(), but
"show-branch --reflog" will pretty reliably segfault as it tries to
access the garbage pointer.
Fixing the segfault would be pretty easy. But the off-by-one problem is
inherent in this approach. So let's start by reverting the commit to
give us a clean slate to work with.
This isn't a pure revert; all of the code changes are reverted, but for
the tests:
1. We'll flip the cases in t1508 to expect_failure; making these work
was the goal of 6436a2028, and we'll want to use them for our
replacement approach.
2. There's a test in t3202 for "show-branch --reflog", but it expects
the broken output! It was added by f2463490c4 (show-branch: show
reflog message, 2021-12-02) which was fixing another bug, and I
think the author simply didn't notice that the second line showed
the wrong reflog.
Rather than fixing that test, let's replace it with one that is
more thorough (while still covering the reflog message fix from
that commit). We'll use a longer reflog, which lets us see more
entries (thus making the "off by one" pattern much more clear). And
we'll use a more recent timestamp for "now" so that our relative
dates have more resolution. That lets us see that the reflog dates
are correct (whereas when you are 4 years away, two entries that
are 60 seconds apart will have the same "4 years ago" relative
date). Because we're adjusting the repository state, I've moved
this new test to the end of the script, leaving the other tests
undisturbed.
We'll also add a new test which covers the missing reflog case;
previously it segfaulted, but now it reports the empty reflog).
Reported-by: Yasushi SHOJI <yasushi.shoji@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 583b7ea31b (upload-pack/fetch-pack: support side-band
communication, 2006-06-21) the abort message sent by upload-pack in
case of possible repository corruption ends with a null character.
This can be seen in several test cases in 't5530-upload-pack-error.sh'
where 'grep <pattern> output.err' often reports "Binary file
output.err matches" because of that null character.
The reason for this is that the abort message is defined as a string
literal, and we pass its size to the send function as
sizeof(abort_msg), which also counts the terminating null character.
Use strlen() instead to avoid sending that terminating null character.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
