Commit Graph

65333 Commits

Author SHA1 Message Date
Junio C Hamano
b0b5337876 Merge branch 'jk/t5319-midx-corruption-test-deflake'
Test fix.

* jk/t5319-midx-corruption-test-deflake:
  t5319: corrupt more bytes of the midx checksum
2021-12-10 14:35:08 -08:00
Junio C Hamano
9b0a970ace Merge branch 'js/trace2-avoid-recursive-errors'
trace2 error code path fix.

* js/trace2-avoid-recursive-errors:
  trace2: disable tr2_dst before warning on write errors
2021-12-10 14:35:08 -08:00
Junio C Hamano
2d5b70de2d Merge branch 'jt/pack-header-lshift-overflow'
The code to decode the length of packed object size has been
corrected.

* jt/pack-header-lshift-overflow:
  packfile: avoid overflowing shift during decode
2021-12-10 14:35:08 -08:00
Junio C Hamano
a0f3df5d64 Merge branch 'jk/jump-merge-with-pathspec'
The "merge" subcommand of "git jump" (in contrib/) silently ignored
pathspec and other parameters.

* jk/jump-merge-with-pathspec:
  git-jump: pass "merge" arguments to ls-files
2021-12-10 14:35:08 -08:00
Junio C Hamano
a9c84980d0 Merge branch 'jk/test-bitmap-fix'
Tighten code for testing pack-bitmap.

* jk/test-bitmap-fix:
  test_bitmap_hashes(): handle repository without bitmaps
2021-12-10 14:35:08 -08:00
Junio C Hamano
d1305bd3cf Merge branch 'ab/generate-command-list'
Build optimization.

* ab/generate-command-list:
  generate-cmdlist.sh: don't parse command-list.txt thrice
  generate-cmdlist.sh: replace "grep' invocation with a shell version
  generate-cmdlist.sh: do not shell out to "sed"
  generate-cmdlist.sh: stop sorting category lines
  generate-cmdlist.sh: replace for loop by printf's auto-repeat feature
  generate-cmdlist.sh: run "grep | sort", not "sort | grep"
  generate-cmdlist.sh: don't call get_categories() from category_list()
  generate-cmdlist.sh: spawn fewer processes
  generate-cmdlist.sh: trivial whitespace change
  command-list.txt: sort with "LC_ALL=C sort"
2021-12-10 14:35:08 -08:00
Junio C Hamano
03194a1afa Merge branch 'tw/var-default-branch'
"git var GIT_DEFAULT_BRANCH" is a way to see what name is used for
the newly created branch if "git init" is run.

* tw/var-default-branch:
  var: add GIT_DEFAULT_BRANCH variable
2021-12-10 14:35:07 -08:00
Junio C Hamano
1c39c822a9 Merge branch 'jk/strbuf-addftime-seconds-since-epoch'
The "--date=format:<strftime>" gained a workaround for the lack of
system support for a non-local timezone to handle "%s" placeholder.

* jk/strbuf-addftime-seconds-since-epoch:
  strbuf_addftime(): handle "%s" manually
2021-12-10 14:35:07 -08:00
Junio C Hamano
bd16b3c39f Merge branch 'js/ci-no-directional-formatting'
CI has been taught to catch some Unicode directional formatting
sequence that can be used in certain mischief.

* js/ci-no-directional-formatting:
  ci: disallow directional formatting
2021-12-10 14:35:06 -08:00
Junio C Hamano
3d2dce168f Merge branch 'jc/fix-first-object-walk'
Doc update.

* jc/fix-first-object-walk:
  docs: add headers in MyFirstObjectWalk
  docs: fix places that break compilation in MyFirstObjectWalk
2021-12-10 14:35:05 -08:00
Junio C Hamano
b5e7f5e5b1 Merge branch 'if/redact-packfile-uri'
Redact the path part of packfile URI that appears in the trace output.

* if/redact-packfile-uri:
  http-fetch: redact url on die() message
  fetch-pack: redact packfile urls in traces
2021-12-10 14:35:04 -08:00
Junio C Hamano
23c83fc473 Merge branch 'ja/doc-cleanup'
Doc update.

* ja/doc-cleanup:
  init doc: --shared=0xxx does not give umask but perm bits
  doc: git-init: clarify file modes in octal.
  doc: git-http-push: describe the refs as pattern pairs
  doc: uniformize <URL> placeholders' case
  doc: use three dots for indicating repetition instead of star
  doc: git-ls-files: express options as optional alternatives
  doc: use only hyphens as word separators in placeholders
  doc: express grammar placeholders between angle brackets
  doc: split placeholders as individual tokens
  doc: fix git credential synopsis
2021-12-10 14:35:03 -08:00
Junio C Hamano
6d1e149ac0 Merge branch 'gc/remote-with-fewer-static-global-variables'
Code clean-up to eventually allow information on remotes defined
for an arbitrary repository to be read.

* gc/remote-with-fewer-static-global-variables:
  remote: die if branch is not found in repository
  remote: remove the_repository->remote_state from static methods
  remote: use remote_state parameter internally
  remote: move static variables into per-repository struct
  t5516: add test case for pushing remote refspecs
2021-12-10 14:35:02 -08:00
Junio C Hamano
5396d7b298 Merge branch 'vd/sparse-sparsity-fix-on-read'
Ensure that the sparseness of the in-core index matches the
index.sparse configuration specified by the repository immediately
after the on-disk index file is read.

* vd/sparse-sparsity-fix-on-read:
  sparse-index: update do_read_index to ensure correct sparsity
  sparse-index: add ensure_correct_sparsity function
  sparse-index: avoid unnecessary cache tree clearing
  test-read-cache.c: prepare_repo_settings after config init
2021-12-10 14:35:01 -08:00
Junio C Hamano
83113c4268 Merge branch 'cw/protocol-v2-doc-fix'
Doc update.

* cw/protocol-v2-doc-fix:
  protocol-v2.txt: align delim-pkt spec with usage
2021-12-10 14:35:00 -08:00
Fabian Stelzer
50992f96c5 ssh signing: verify ssh-keygen in test prereq
Do a full ssh signing, find-principals and verify operation in the test
prereq's to make sure ssh-keygen works as expected. Only generating the
keys and verifying its presence is not sufficient in some situations.
One example was ssh-keygen creating unusable ssh keys in cygwin because
of unsafe default permissions for the key files. The other a broken
openssh 8.7 that segfaulted on any find-principals operation. This
extended prereq check avoids future test breakages in case ssh-keygen or
any environment behaviour changes.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
122842fd93 ssh signing: make fmt-merge-msg consider key lifetime
Set the payload_type for check_signature() when generating merge messages to
verify merged tags signatures key lifetimes.
Implements the same tests as for verify-commit.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
dd3aa418aa ssh signing: make verify-tag consider key lifetime
Set the payload_type for check_signature() when calling verify-tag.
Implements the same tests as for verify-commit.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
4bbf3780ff ssh signing: make git log verify key lifetime
Set the payload_type for check_signature() when calling git log.
Implements the same tests as for verify-commit.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
6393c956f4 ssh signing: make verify-commit consider key lifetime
If valid-before/after dates are configured for this signatures key in the
allowedSigners file then the verification should check if the key was valid at
the time the commit was made. This allows for graceful key rollover and
revoking keys without invalidating all previous commits.
This feature needs openssh > 8.8. Older ssh-keygen versions will simply
ignore this flag and use the current time.
Strictly speaking this feature is available in 8.7, but since 8.7 has a
bug that makes it unusable in another needed call we require 8.8.

Timestamp information is present on most invocations of check_signature.
However signer ident is not. We will need the signer email / name to be able
to implement "Trust on first use" functionality later.
Since the payload contains all necessary information we can parse it
from there. The caller only needs to provide us some info about the
payload by setting payload_type in the signature_check struct.

 - Add payload_type field & enum and payload_timestamp to struct
   signature_check
 - Populate the timestamp when not already set if we know about the
   payload type
 - Pass -Overify-time={payload_timestamp} in the users timezone to all
   ssh-keygen verification calls
 - Set the payload type when verifying commits
 - Add tests for expired, not yet valid and keys having a commit date
   outside of key validity as well as within

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
30770aa981 ssh signing: add key lifetime test prereqs
if ssh-keygen supports -Overify-time, add test keys marked as expired,
not yet valid and valid both within the test_tick timeframe and outside of it.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
02769437e1 ssh signing: use sigc struct to pass payload
To be able to extend the payload metadata with things like its creation
timestamp or the creators ident we remove the payload parameters to
check_signature() and use the already existing sigc->payload field
instead, only adding the length field to the struct. This also allows
us to get rid of the xmemdupz() calls in the verify functions. Since
sigc is now used to input data as well as output the result move it to
the front of the function list.

 - Add payload_length to struct signature_check
 - Populate sigc.payload/payload_len on all call sites
 - Remove payload parameters to check_signature()
 - Remove payload parameters to internal verify_* functions and use sigc
   instead
 - Remove xmemdupz() used for verbose output since payload is now already
   populated.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
cafd34522f t/fmt-merge-msg: make gpgssh tests more specific
Some GPGSSH fmt-merge-msg tests were only grepping for failed/successful
signature validation and not checking for the tag in the resulting merge
message. Add the missing grep for it.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Fabian Stelzer
5a2c1c0dee t/fmt-merge-msg: do not redirect stderr
All the GPG and GPGSSH tests are redirecing stdout as well as stderr
to `actual` and grep for success/failure over the resulting file.
However, no output is printed on stderr and we do not need to
include it in the grep.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:03 -08:00
Eric Wong
2c68f577fc cbtree: remove broken and unused cb_unlink
cb_unlink is broken once a node is no longer self-referential
due to subsequent insertions.  This is a consequence of an
intrusive implementation and I'm not sure if it's easily fixable
while retaining our cache-friendly intrusive property (I've
tried for several hours in another project).

In any case, we're not using cb_unlink anywhere in our codebase,
just get rid of it to avoid misleading future readers.

Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 15:18:35 -08:00
Han-Wen Nienhuys
9912391402 t1430: create valid symrefs using test-helper
This still leaves some other direct filesystem access. Currently, the files
backend does not allow invalidly named symrefs. Fixes for this are currently in
the 'seen' branch

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:20 -08:00
Han-Wen Nienhuys
e39ceeb475 t1430: remove refs using test-tool
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:19 -08:00
Han-Wen Nienhuys
3c966c7b4e refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
Use this flag with the test-helper in t1430, to avoid direct writes to the ref
database.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:19 -08:00
Han-Wen Nienhuys
e9706a188f refs: introduce REF_SKIP_OID_VERIFICATION flag
This lets the ref-store test helper write non-existent or unparsable objects
into the ref storage.

Use this to make t1006 and t3800 independent of the files storage backend.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:19 -08:00
Han-Wen Nienhuys
0464d0a134 refs: update comment.
REF_IS_PRUNING is right below this comment, so it clearly does not belong in
this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017
"refs: tidy up and adjust visibility of the `ref_update` flags").

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:19 -08:00
Han-Wen Nienhuys
df25a19d72 test-ref-store: plug memory leak in cmd_delete_refs
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:19 -08:00
Han-Wen Nienhuys
cd2d40fb7f test-ref-store: parse symbolic flag constants
This lets tests use REF_XXXX constants instead of hardcoded integers. The flag
names should be separated by a ','.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:18 -08:00
Han-Wen Nienhuys
93db6eef04 test-ref-store: remove force-create argument for create-reflog
Nobody uses force_create=0, so this flag is unnecessary.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 13:15:18 -08:00
Ævar Arnfjörð Bjarmason
eafd6e7e55 object.c: use BUG(...) no die("BUG: ...") in lookup_object_by_type()
Adjust code added in 7463064b28 (object.h: add
lookup_object_by_type() function, 2021-06-22) to use the BUG()
function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 12:33:58 -08:00
Ævar Arnfjörð Bjarmason
a78537a0f2 pathspec: use BUG(...) not die("BUG:%s:%d....", <file>, <line>)
Change code that was added in 8f4f8f4579 (guard against new pathspec
magic in pathspec matching code, 2013-07-14) to use the BUG() macro
instead of emitting a "fatal" message with the "__FILE__"-name and
"__LINE__"-numbers.

The original code predated the existence of the BUG() function, which
was added in d8193743e0 (usage.c: add BUG() function, 2017-05-12).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 12:31:17 -08:00
Ævar Arnfjörð Bjarmason
46d699f492 strbuf.h: use BUG(...) not die("BUG: ...")
In 7141efab24 (strbuf: clarify assertion in strbuf_setlen(),
2011-04-27) this 'die("BUG: "' invocation was added with the rationale
that strbuf.c had existing users doing the same, but those users were
later changed to use BUG() in 033abf97fc (Replace all die("BUG: ...")
calls by BUG() ones, 2018-05-02). Let's do the same here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 12:31:16 -08:00
Ævar Arnfjörð Bjarmason
5867757d88 pack-objects: use BUG(...) not die("BUG: ...")
Change this code added in da93d12b00 (pack-objects: be incredibly
anal about stdio semantics, 2006-04-02) to use BUG() instead.

See 1a07e59c3e (Update messages in preparation for i18n, 2018-07-21)
for when the "BUG: " prefix was added, and [1] for background on the
Solaris behavior that prompted the exhaustive error checking in this
fgets() loop.

1. https://lore.kernel.org/git/824.1144007555@lotus.CS.Berkeley.EDU/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-07 12:31:16 -08:00
Derrick Stolee
8c5de0d265 unpack-trees: use traverse_path instead of name
The sparse_dir_matches_path() method compares a cache entry that is a
sparse directory entry against a 'struct traverse_info *info' and a
'struct name_entry *p' to see if the cache entry has exactly the right
name for those other inputs.

This method was introduced in 523506d (unpack-trees: unpack sparse
directory entries, 2021-07-14), but included a significant mistake. The
path comparisons used 'info->name' instead of 'info->traverse_path'.
Since 'info->name' only stores a single tree entry name while
'info->traverse_path' stores the full path from root, this method does
not work when 'info' is in a subdirectory of a directory. Replacing the
right strings and their corresponding lengths make the method work
properly.

The previous change included a failing test that exposes this issue.
That test now passes. The critical detail is that as we go deep into
unpack_trees(), the logic for merging a sparse directory entry with a
tree entry during 'git checkout' relies on this
sparse_dir_matches_path() in order to avoid calling
traverse_trees_recursive() during unpack_callback() in this hunk:

	if (!is_sparse_directory_entry(src[0], names, info) &&
	    traverse_trees_recursive(n, dirmask, mask & ~dirmask,
					    names, info) < 0) {
		return -1;
	}

For deep paths, the short-circuit never occurred and
traverse_trees_recursive() was being called incorrectly and that was
causing other strange issues. Specifically, the error message from the
now-passing test previously included this:

      error: Your local changes to the following files would be overwritten by checkout:
              deep/deeper1/deepest2/a
              deep/deeper1/deepest3/a
      Please commit your changes or stash them before you switch branches.
      Aborting

These messages occurred because the 'current' cache entry in
twoway_merge() was showing as NULL because the index did not contain
entries for the paths contained within the sparse directory entries. We
instead had 'oldtree' given as the entry at HEAD and 'newtree' as the
entry in the target tree. This led to reject_merge() listing these
paths.

Now that sparse_dir_matches_path() works the same for deep paths as it
does for shallow depths, the rest of the logic kicks in to properly
handle modifying the sparse directory entries as designed.

Reported-by: Gustave Granroth <gus.gran@gmail.com>
Reported-by: Mike Marcelais <michmarc@exchange.microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-06 09:24:54 -08:00
Derrick Stolee
1b38efc7a0 t1092: add deeper changes during a checkout
Extend the repository data in the setup of t1092 to include more
directories within two parent directories. This reproduces a bug found
by users of the sparse index feature with suitably-complicated
sparse-checkout definitions.

Add a failing test that fails in its first 'git checkout deepest' run in
the sparse index case with this error:

  error: Your local changes to the following files would be overwritten by checkout:
          deep/deeper1/deepest2/a
          deep/deeper1/deepest3/a
  Please commit your changes or stash them before you switch branches.
  Aborting

The next change will fix this error, and that fix will make it clear why
the extra depth is necessary for revealing this bug. The assignment of
the sparse-checkout definition to include deep/deeper1/deepest as a
sibling directory is important to ensure that deep/deeper1 is not a
sparse directory entry, but deep/deeper1/deepest2 is.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-06 09:24:53 -08:00
Junio C Hamano
91028f7659 grep: clarify what grep.patternType=default means
We documented that with grep.patternType set to default, the "git
grep" command returns to "the default matching behavior" in 84befcd0
(grep: add a grep.patternType configuration setting, 2012-08-03).

The grep.extendedRegexp configuration variable was the only way to
configure the behavior before that, after b22520a3 (grep: allow -E
and -n to be turned on by default via configuration, 2011-03-30)
introduced it.

It is understandable that we referred to the behavior that honors
the older configuration variable as "the default matching"
behavior.  It is fairly clear in its log message:

    When grep.patternType is set to a value other than "default", the
    grep.extendedRegexp setting is ignored. The value of "default" restores
    the current default behavior, including the grep.extendedRegexp
    behavior.

But when the paragraph is read in isolation by a new person who is
not aware of that backstory (which is the synonym for "most users"),
the "default matching behavior" can be read as "how 'git grep'
behaves without any configuration variables or options", which is
"match the pattern as BRE".

Clarify what the passage means by elaborating what the phrase
"default matching behavior" wanted to mean.

Helped-by: Johannes Altmanninger <aclopte@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-05 12:26:43 -08:00
Johannes Schindelin
9f3547837e tests: set GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME only when needed
A couple of test scripts have actually been adapted to accommodate for a
configurable default branch name, but they still overrode it via the
`GIT_TEST_*` variable. Let's drop that override where possible.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-05 11:34:28 -08:00
Elijah Newren
434e0636db sequencer: do not export GIT_DIR and GIT_WORK_TREE for 'exec'
Commands executed from `git rebase --exec` can give different behavior
from within that environment than they would outside of it, due to the
fact that sequencer.c exports both GIT_DIR and GIT_WORK_TREE.  For
example, if the relevant script calls something like

  git -C ../otherdir log --format=%H --no-walk

the user may be surprised to find that the command above does not show a
commit hash from ../otherdir, because $GIT_DIR prevents automatic gitdir
detection and makes the -C option useless.

This is a regression in behavior from the original legacy
implemented-in-shell rebase.  It is perhaps rare for it to cause
problems in practice, especially since most small problems that were
caused by this area of bugs has been fixed-up in the past in a way that
masked the particular bug observed without fixing the real underlying
problem.

An explanation of how we arrived at the current situation is perhaps
merited.  The setting of GIT_DIR and GIT_WORK_TREE done by sequencer.c
arose from a sequence of historical accidents:

* When rebase was implemented as a shell command, it would call
  git-sh-setup, which among other things would set GIT_DIR -- but not
  export it.  This meant that when rebase --exec commands were run via
      /bin/sh -c "$COMMAND"
  they would not inherit the GIT_DIR setting.  The fact that GIT_DIR
  was not set in the run $COMMAND is the behavior we'd like to restore.

* When the rebase--helper builtin was introduced to allow incrementally
  replacing shell with C code, we had an implementation that was half
  shell, half C.  In particular, commit 18633e1a22 ("rebase -i: use the
  rebase--helper builtin", 2017-02-09) added calls to
      exec git rebase--helper ...
  which caused rebase--helper to inherit the GIT_DIR environment
  variable from the shell.  git's setup would change the environment
  variable from an absolute path to a relative one (".git"), but would
  leave it set.  This meant that when rebase --exec commands were run
  via
      run_command_v_opt(...)
  they would inherit the GIT_DIR setting.

* In commit 09d7b6c6fa ("sequencer: pass absolute GIT_DIR to exec
  commands", 2017-10-31), it was noted that the GIT_DIR caused problems
  with some commands; e.g.
      git rebase --exec 'cd subdir && git describe' ...
  would have GIT_DIR=.git which was invalid due to the change to the
  subdirectory.  Instead of questioning why GIT_DIR was set, that commit
  instead made sequencer change GIT_DIR to be an absolute path and
  explicitly export it via
      argv_array_pushf(&child_env, "GIT_DIR=%s", absolute_path(get_git_dir()));
      run_command_v_opt_cd_env(..., child_env.argv)

* In commit ab5e67d751 ("sequencer: pass absolute GIT_WORK_TREE to exec
  commands", 2018-07-14), it was noted that when GIT_DIR is set but
  GIT_WORK_TREE is not, that we do not discover GIT_WORK_TREE but just
  assume it is '.'.  That is incorrect if trying to run commands from a
  subdirectory.  However, rather than question why GIT_DIR was set, that
  commit instead also added GIT_WORK_TREE to the list of things to
  export.

Each of the above problems would have been fixed automatically when
git-rebase became a full builtin, had it not been for the fact that
sequencer.c started exporting GIT_DIR and GIT_WORK_TREE in the interim.
Stop exporting them now.

Signed-off-by: Elijah Newren <newren@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Johannes Altmanninger <aclopte@gmail.com>
Acked-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:41:05 -08:00
Jeff King
1e45db1214 xdiff: drop unused flags parameter from recs_match
Since 6b13bc3232 (xdiff: simplify comparison, 2021-11-17), we do not
call xdl_recmatch() from xdiffi.c's recs_match(), so we no longer need
the "flags" parameter. That in turn lets us drop the flags parameters
from the group-slide functions which call it.

There's no functional change here; it's just making these functions a
little simpler.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:27:53 -08:00
Jeff King
25449450c0 xdiff: drop xpparam_t parameter from histogram cmp_recs()
Since 663c5ad035 (diff histogram: intern strings, 2021-11-17), our
cmp_recs() does not call xdl_recmatch(), and thus no longer needs an
xpparam_t struct from which to get the flags. We can drop the unused
parameter from the function, as well as the macro which wraps it.

There's no functional change here; it's just simplifying things (and
making -Wunused-parameter happier).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:27:53 -08:00
Jeff King
cf0b26d90c xdiff: drop CMP_ENV macro from xhistogram
This macro has been unused since 43ca7530df (xdiff/xhistogram: rely on
xdl_trim_ends(), 2011-08-01). The function that called it went away
there, and its use in the CMP() macro was inlined. It probably should
have been deleted then, but nobody noticed.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:27:53 -08:00
Eric Sunshine
b50252484f git-worktree.txt: add missing -v to synopsis for worktree list
When verbose mode was added to `git worktree list` by 076b444a62
(worktree: teach `list` verbose mode, 2021-01-27), although the
documentation was updated to reflect the new functionality, the
synopsis was overlooked. Correct this minor oversight.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:27:25 -08:00
Eric Sunshine
da8fb6be55 worktree: send "chatty" messages to stderr
The order in which the stdout and stderr streams are flushed is not
guaranteed to be the same across platforms or `libc` implementations.
This lack of determinism can lead to anomalous and potentially confusing
output if normal (stdout) output is flushed after error (stderr) output.
For instance, the following output which clearly indicates a failure due
to a fatal error:

    % git worktree add ../foo bar
    Preparing worktree (checking out 'bar')
    fatal: 'bar' is already checked out at '.../wherever'

has been reported[1] on Microsoft Windows to appear as:

    % git worktree add ../foo bar
    fatal: 'bar' is already checked out at '.../wherever'
    Preparing worktree (checking out 'bar')

which may confuse the reader into thinking that the command somehow
recovered and ran to completion despite the error.

This problem crops up because the "chatty" status message "Preparing
worktree" is sent to stdout, whereas the "fatal" error message is sent
to stderr. One way to fix this would be to flush stdout manually before
git-worktree reports any errors to stderr.

However, common practice in Git is for "chatty" messages to be sent to
stderr. Therefore, a more appropriate fix is to adjust git-worktree to
conform to that practice by sending its "chatty" messages to stderr
rather than stdout as is currently the case.

There may be concern that relocating messages from stdout to stderr
could break existing tooling, however, these messages are already
internationalized, thus are unstable. And, indeed, the "Preparing
worktree" message has already been the subject of somewhat significant
changes in 2c27002a0a (worktree: improve message when creating a new
worktree, 2018-04-24). Moreover, there is existing precedent, such as
68b939b2f0 (clone: send diagnostic messages to stderr, 2013-09-18) which
likewise relocated "chatty" messages from stdout to stderr for
git-clone.

[1]: https://lore.kernel.org/git/CA+34VNLj6VB1kCkA=MfM7TZR+6HgqNi5-UaziAoCXacSVkch4A@mail.gmail.com/T/

Reported-by: Baruch Burstein <bmburstein@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:27:11 -08:00
Taylor Blau
0bf0de6cc7 packfile: make close_pack_revindex() static
Since its definition in 2f4ba2a867 (packfile: prepare for the existence
of '*.rev' files, 2021-01-25), the only caller of
`close_pack_revindex()` was within packfile.c.

Thus there is no need for this to be exposed via packfile.h, and instead
can remain static within packfile.c's compilation unit. While we're
here, move the function's opening brace onto its own line.

Noticed-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 23:01:38 -08:00
Eric Sunshine
e258eb4800 CodingGuidelines: document which output goes to stdout vs. stderr
It has long been practice on this project for a command to emit its
primary output to stdout so that it can be captured to a file or sent
down a pipe, and to emit "chatty" messages (such as those reporting
progress) to stderr so that they don't interfere with the primary
output. However, this practice is not necessarily universal; another
common practice is to send only error messages to stderr, and all other
messages to stdout. Therefore, help newcomers out by documenting how
stdout and stderr are used on this project.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-04 17:26:41 -08:00
Junio C Hamano
44ba10d671 revision: use C99 declaration of variable in for() loop
There are certain C99 features that might be nice to use in our code
base, but we've hesitated to do so in order to avoid breaking
compatibility with older compilers. But we don't actually know if
people are even using pre-C99 compilers these days.

One way to figure that out is to introduce a very small use of a
feature, and see if anybody complains, and we've done so to probe
the portability for a few features like "trailing comma in enum
declaration", "designated initializer for struct", and "designated
initializer for array".  A few years ago, we tried to use a handy

    for (int i = 0; i < n; i++)
	use(i);

to introduce a new variable valid only in the loop, but found that
some compilers we cared about didn't like it back then.  Two years
is a long-enough time, so let's try it again.

If this patch can survive a few releases without complaint, then we
can feel more confident that variable declaration in for() loop is
supported by the compilers our user base use.  And if we do get
complaints, then we'll have gained some data and we can easily
revert this patch.

Helped-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-03 10:16:00 -08:00