Commit Graph

74087 Commits

Author SHA1 Message Date
Junio C Hamano
04f5a52757 Post 2.46-rc0 batch #2
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-16 11:18:58 -07:00
Junio C Hamano
d6c86368c8 Merge branch 'bc/gitfaq-more'
A handful of entries are added to the GitFAQ document.

* bc/gitfaq-more:
  doc: mention that proxies must be completely transparent
  gitfaq: add entry about syncing working trees
  gitfaq: give advice on using eol attribute in gitattributes
  gitfaq: add documentation on proxies
2024-07-16 11:18:58 -07:00
Junio C Hamano
fe5ba894ec Merge branch 'bc/http-proactive-auth'
The http transport can now be told to send request with
authentication material without first getting a 401 response.

* bc/http-proactive-auth:
  http: allow authenticating proactively
2024-07-16 11:18:57 -07:00
Junio C Hamano
12d49fd028 Merge branch 'jc/where-is-bash-for-ci'
Shell script clean-up.

* jc/where-is-bash-for-ci:
  ci: unify bash calling convention
2024-07-16 11:18:57 -07:00
Junio C Hamano
5d71940dda Merge branch 'ds/advice-sparse-index-expansion'
A new warning message is issued when a command has to expand a
sparse index to handle working tree cruft that are outside of the
sparse checkout.

* ds/advice-sparse-index-expansion:
  advice: warn when sparse index expands
2024-07-16 11:18:56 -07:00
Junio C Hamano
f4c6a0e275 Merge branch 'cb/send-email-sanitize-trailer-addresses'
Address-looking strings found on the trailer are now placed on the
Cc: list after running through sanitize_address by "git send-email".

* cb/send-email-sanitize-trailer-addresses:
  git-send-email: use sanitized address when reading mbox body
2024-07-16 11:18:56 -07:00
Junio C Hamano
ffc8f1142c Merge branch 'en/ort-inner-merge-error-fix'
The "ort" merge backend saw one bugfix for a crash that happens
when inner merge gets killed, and assorted code clean-ups.

* en/ort-inner-merge-error-fix:
  merge-ort: fix missing early return
  merge-ort: convert more error() cases to path_msg()
  merge-ort: upon merge abort, only show messages causing the abort
  merge-ort: loosen commented requirements
  merge-ort: clearer propagation of failure-to-function from merge_submodule
  merge-ort: fix type of local 'clean' var in handle_content_merge ()
  merge-ort: maintain expected invariant for priv member
  merge-ort: extract handling of priv member into reusable function
2024-07-16 11:18:55 -07:00
Christian Hesse
730914ed7e refs: correct the version numbers in a comment
The paragraph talks about a change made in c8f815c2 (refs: remove
functions without ref store, 2024-05-07), which is v2.46.0-rc0~119^2
and will be published as part of v2.46, not v2.45.

Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-16 09:06:22 -07:00
Junio C Hamano
ad850ef1cf Post 2.46-rc0 batch #1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-15 10:11:44 -07:00
Junio C Hamano
9118e46e81 Merge branch 'cp/unit-test-reftable-record'
A test in reftable library has been rewritten using the unit test
framework.

* cp/unit-test-reftable-record:
  t-reftable-record: add tests for reftable_log_record_compare_key()
  t-reftable-record: add tests for reftable_ref_record_compare_name()
  t-reftable-record: add index tests for reftable_record_is_deletion()
  t-reftable-record: add obj tests for reftable_record_is_deletion()
  t-reftable-record: add log tests for reftable_record_is_deletion()
  t-reftable-record: add ref tests for reftable_record_is_deletion()
  t-reftable-record: add comparison tests for obj records
  t-reftable-record: add comparison tests for index records
  t-reftable-record: add comparison tests for ref records
  t-reftable-record: add reftable_record_cmp() tests for log records
  t: move reftable/record_test.c to the unit testing framework
2024-07-15 10:11:44 -07:00
Junio C Hamano
f582dc3c5a Merge branch 'jc/disable-push-nego-for-deletion'
"git push" that pushes only deletion gave an unnecessary and
harmless error message when push negotiation is configured, which
has been corrected.

* jc/disable-push-nego-for-deletion:
  push: avoid showing false negotiation errors
2024-07-15 10:11:43 -07:00
Junio C Hamano
fbeed643b9 Merge branch 'ri/doc-show-branch-fix'
Docfix.

* ri/doc-show-branch-fix:
  doc: fix the max number of branches shown by "show-branch"
2024-07-15 10:11:43 -07:00
Junio C Hamano
d319ad5704 Merge branch 'tb/dev-build-pedantic-fix'
Developer build procedure fix.

* tb/dev-build-pedantic-fix:
  config.mak.dev: fix typo when enabling -Wpedantic
2024-07-15 10:11:42 -07:00
Junio C Hamano
76f49679b1 Merge branch 'rs/clang-format-updates'
Custom control structures we invented more recently have been
taught to the clang-format file.

* rs/clang-format-updates:
  clang-format: include kh_foreach* macros in ForEachMacros
2024-07-15 10:11:42 -07:00
Junio C Hamano
ccb74f51c9 Merge branch 'am/gitweb-feed-use-committer-date'
GitWeb update to use committer date consistently in rss/atom feeds.

* am/gitweb-feed-use-committer-date:
  gitweb: rss/atom change published/updated date to committer date
2024-07-15 10:11:41 -07:00
Junio C Hamano
820e796984 Merge branch 'jk/tests-without-dns'
Test suite has been taught not to unnecessarily rely on DNS failing
a bogus external name.

* jk/tests-without-dns:
  t/lib-bundle-uri: use local fake bundle URLs
  t5551: do not confirm that bogus url cannot be used
  t5553: use local url for invalid fetch
2024-07-15 10:11:41 -07:00
Junio C Hamano
cda729581b Merge branch 'gt/unit-test-oidmap'
An existing test of oidmap API has been rewritten with the
unit-test framework.

* gt/unit-test-oidmap:
  t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c
2024-07-15 10:11:40 -07:00
Junio C Hamano
b227482ea0 Merge branch 'as/describe-broken-refresh-index-fix'
"git describe --dirty --broken" forgot to refresh the index before
seeing if there is any chang, ("git describe --dirty" correctly did
so), which has been corrected.

* as/describe-broken-refresh-index-fix:
  describe: refresh the index when 'broken' flag is used
2024-07-15 10:11:40 -07:00
Junio C Hamano
d8b9b1fc81 Merge branch 'rj/t0613-no-longer-leaks'
A test that no longer leaks has been marked as such.

* rj/t0613-no-longer-leaks:
  t0613: mark as leak-free
2024-07-15 10:11:39 -07:00
Junio C Hamano
84fc58f24b Merge branch 'rj/t0612-no-longer-leaks'
A test that no longer leaks has been marked as such.

* rj/t0612-no-longer-leaks:
  t0612: mark as leak-free
2024-07-15 10:11:39 -07:00
Johannes Schindelin
9ed143ee33 var(win32): do report the GIT_SHELL_PATH that is actually used
On Windows, Unix-like paths like `/bin/sh` make very little sense. In
the best case, they simply don't work, in the worst case they are
misinterpreted as absolute paths that are relative to the drive
associated with the current directory.

To that end, Git does not actually use the path `/bin/sh` that is
recorded e.g. when `run_command()` is called with a Unix shell
command-line. Instead, as of 776297548e (Do not use SHELL_PATH from
build system in prepare_shell_cmd on Windows, 2012-04-17), it
re-interprets `/bin/sh` as "look up `sh` on the `PATH` and use the
result instead".

This is the logic users expect to be followed when running `git var
GIT_SHELL_PATH`.

However, when 1e65721227 (var: add support for listing the shell,
2023-06-27) introduced support for `git var GIT_SHELL_PATH`, Windows was
not special-cased as above, which is why it outputs `/bin/sh` even
though that disagrees with what Git actually uses.

Let's fix this by using the exact same logic as `prepare_shell_cmd()`,
adjusting the Windows-specific `git var GIT_SHELL_PATH` test case to
verify that it actually finds a working executable.

Reported-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:37 -07:00
Johannes Schindelin
877da5e208 run-command: declare the git_shell_path() function globally
The intention is to use it in `git var GIT_SHELL_PATH`, therefore we
need this function to stop being file-local only.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:37 -07:00
Johannes Schindelin
92fe7c7d42 run-command(win32): resolve the path to the Unix shell early
In 776297548e (Do not use SHELL_PATH from build system in
prepare_shell_cmd on Windows, 2012-04-17), the hard-coded path to the
Unix shell was replaced by passing `sh` instead when executing Unix
shell scripts in Git.

This was done because the hard-coded path to the Unix shell is incorrect
on Windows because it not only is a Unix-style absolute path instead of
a Windows one, but Git uses the runtime prefix feature on Windows, i.e.
the correct path cannot be hard-coded.

Naturally, the `sh` argument will be resolved to the full path of said
executable eventually.

To help fixing the bug where `git var GIT_SHELL_PATH` currently does not
reflect that logic, but shows that incorrect hard-coded Unix-style
absolute path, let's resolve the full path to the `sh` executable early
in the `git_shell_path()` function so that we can use it in `git var`,
too, and be sure that the output is equivalent to what `run_command()`
does when it is asked to execute a command-line using a Unix shell.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:37 -07:00
Johannes Schindelin
f1ed769a3b mingw(is_msys2_sh): handle forward slashes in the sh.exe path, too
Whether the full path to the MSYS2 Bash is specified using backslashes
or forward slashes, in either case the command-line arguments need to be
quoted in the MSYS2-specific manner instead of using regular Win32
command-line quoting rules.

In preparation for `prepare_shell_cmd()` to use the full path to
`sh.exe` (with forward slashes for consistency), let's teach the
`is_msys2_sh()` function about this; Otherwise 5580.4 'clone with
backslashed path' would fail once `prepare_shell_cmd()` uses the full
path instead of merely `sh`.

This patch relies on the just-introduced fix where `fspathcmp()` handles
backslashes and forward slashes as equivalent on Windows.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:37 -07:00
Johannes Schindelin
193eda7507 win32: override fspathcmp() with a directory separator-aware version
On Windows, the backslash is the directory separator, even if the
forward slash can be used, too, at least since Windows NT.

This means that the paths `a/b` and `a\b` are equivalent, and
`fspathcmp()` needs to be made aware of that fact.

Note that we have to override both `fspathcmp()` and `fspathncmp()`, and
the former cannot be a mere pre-processor constant that transforms calls
to `fspathcmp(a, b)` into `fspathncmp(a, b, (size_t)-1)` because the
function `report_collided_checkout()` in `unpack-trees.c` wants to
assign `list.cmp = fspathcmp`.

Also note that `fspatheq()` does _not_ need to be overridden because it
calls `fspathcmp()` internally.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:36 -07:00
Johannes Schindelin
ce68178a0a strvec: declare the strvec_push_nodup() function globally
This function differs from `strvec_push()` in that it takes ownership of
the allocated string that is passed as second argument.

This is useful when appending elements to the string array that have
been freshly allocated and serve no further other purpose after that.

Without declaring this function globally, call sites would allocate the
memory, only to have `strvec_push()` duplicate the string, and then the
first copy would need to be released. Having this function globally
avoids that kind of unnecessary work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:36 -07:00
Johannes Schindelin
0593c1ea30 run-command: refactor getting the Unix shell path into its own function
This encapsulates the platform-specific logic better.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-13 16:23:36 -07:00
Johannes Schindelin
872721538c cmake: fix build of t-oidtree
When the `oidtree` test helper was turned into a unit test, a new
`lib-oid` source file was added as dependency. This was only done in the
Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed54840872 (t/: migrate helper/test-oidtree.c
to unit-tests/t-oidtree.c, 2024-06-08).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12 14:32:52 -07:00
Karthik Nayak
757c6ee7a3 builtin/push: call set_refspecs after validating remote
When an end-user runs "git push" with an empty string for the remote
repository name, e.g.

    $ git push '' main

"git push" fails with a BUG(). Even though this is a nonsense request
that we want to fail, we shouldn't hit a BUG().  Instead we want to give
a sensible error message, e.g., 'bad repository'".

This is because since 9badf97c42 (remote: allow resetting url list,
2024-06-14), we reset the remote URL if the provided URL is empty. When
a user of 'remotes_remote_get' tries to fetch a remote with an empty
repo name, the function initializes the remote via 'make_remote'. But
the remote is still not a valid remote, since the URL is empty, so it
tries to add the URL alias using 'add_url_alias'. This in-turn will call
'add_url', but since the URL is empty we call 'strvec_clear' on the
`remote->url`. Back in 'remotes_remote_get', we again check if the
remote is valid, which fails, so we return 'NULL' for the 'struct
remote *' value.

The 'builtin/push.c' code, calls 'set_refspecs' before validating the
remote. This worked with empty repo names earlier since we would get a
remote, albeit with an empty URL. With the new changes, we get a 'NULL'
remote value, this causes the check for remote to fail and raises the
BUG in 'set_refspecs'.

Do a simple fix by doing remote validation first. Also add a test to
validate the bug fix. With this, we can also now directly pass remote to
'set_refspecs' instead of it trying to lazily obtain it.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12 09:14:11 -07:00
Junio C Hamano
a7dae3bdc8 Git 2.46-rc0
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-12 08:41:58 -07:00
Junio C Hamano
e6ae4d6efe Merge branch 'rs/simplify-submodule-helper-super-prefix-invocation'
Code clean-up.

* rs/simplify-submodule-helper-super-prefix-invocation:
  submodule--helper: use strvec_pushf() for --super-prefix
2024-07-12 08:41:58 -07:00
Junio C Hamano
7c01dcd018 Merge branch 'as/pathspec-h-typofix'
Typofix.

* as/pathspec-h-typofix:
  pathspec: fix typo "glossary-context.txt" -> "glossary-content.txt"
2024-07-12 08:41:57 -07:00
Piotr Szlazak
8d20119551 doc: update http.cookieFile with in-memory cookie processing
Documentation only mentions how to read cookies from the given file
and how to save them to the file using http.saveCookies.

But underlying libcURL allows the HTTP cookies used only in memory;
cookies from the server will be accepted and sent back in successive
requests within same connection, by using an empty string as the
filename.  Document this.

Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-11 08:50:30 -07:00
Rubén Justo
8c1d6691bc test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default
As we currently describe in t/README, it can happen that:

    Some tests run "git" (or "test-tool" etc.) without properly checking
    the exit code, or git will invoke itself and fail to ferry the
    abort() exit code to the original caller.

Therefore, GIT_TEST_SANITIZE_LEAK_LOG=true is needed to be set to
capture all memory leaks triggered by our tests.

It seems unnecessary to force users to remember this option, as
forgetting it could lead to missed memory leaks.

We could solve the problem by making it "true" by default, but that
might suggest we think "false" makes sense, which isn't the case.

Therefore, the best approach is to remove the option entirely while
maintaining the capability to detect memory leaks in blind spots of our
tests.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-11 08:37:44 -07:00
Jeff King
55fe61559e t/.gitattributes: ignore whitespace in chainlint expect files
The ".expect" files in t/chainlint/ are snippets of expected output from
the chainlint script, and do not necessarily conform to our usual code
style. Especially with the recent change to retain line numbers, blank
lines in the input script end up with trailing whitespace as we print
"3 " for line 3, for example. The point of these files is to match the
output verbatim, so let's not complain about the trailing spaces.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:15:40 -07:00
Jeff King
f6b75726b2 t: convert some here-doc test bodies
The t1404 script checks a lot of output from Git which contains single
quotes. Because the test snippets are themselves wrapped in the same
single-quotes, we have to resort to using $SQ to match them.  This is
error-prone and makes the tests harder to read.

Instead, let's use the new here-doc feature added in the previous
commit, which lets us write anything in the test body we want (except
the here-doc end marker on a line by itself, of course).

Note that we do use "\" in our marker to avoid interpolation (which is
the whole point). But we don't use "<<-", as we want to preserve
whitespace in the snippet (and running with "-v" before and after shows
that we produce the exact same output, except with the ugly $SQ
references fixed).

I just converted every test here, even though only some of them use
$SQ. But it would be equally correct to mix-and-match styles if we don't
mind the inconsistency.

I've also converted a few tests in t0600 which were moved from t1404 (I
had written this patch before they were moved, but it seemed worth
porting over the changes rather than losing them).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:23 -07:00
Jeff King
1d133ae91f test-lib: allow test snippets as here-docs
Most test snippets are wrapped in single quotes, like:

  test_expect_success 'some description' '
          do_something
  '

This sometimes makes the snippets awkward to write, because you can't
easily use single quotes within them. We sometimes work around this with
$SQ, or by loosening regexes to use "." instead of a literal quote, or
by using double quotes when we'd prefer to use single-quotes (and just
adding extra backslash-escapes to avoid interpolation).

This commit adds another option: feeding the snippet via the function's
stdin. This doesn't conflict with anything the snippet would want to do,
because we always redirect its stdin from /dev/null anyway (which we'll
continue to do).

A few notes on the implementation:

  - it would be nice to push this down into test_run_, but we can't, as
    test_expect_success and test_expect_failure want to see the actual
    script content to report it for verbose-mode. A helper function
    limits the amount of duplication in those callers here.

  - The helper function is a little awkward to call, as you feed it the
    name of the variable you want to set. The more natural thing in
    shell would be command substitution like:

      body=$(body_or_stdin "$2")

    but that loses trailing whitespace. There are tricks around this,
    like:

      body=$(body_or_stdin "$2"; printf .)
      body=${body%.}

    but we'd prefer to keep such tricks in the helper, not in each
    caller.

  - I implemented the helper using a sequence of "read" calls. Together
    with "-r" and unsetting the IFS, this preserves incoming whitespace.
    An alternative is to use "cat" (which then requires the gross "."
    trick above). But this saves us a process, which is probably a good
    thing. The "read" builtin does use more read() syscalls than
    necessary (one per byte), but that is almost certainly a win over a
    separate process.

    Both are probably slower than passing a single-quoted string, but
    the difference is lost in the noise for a script that I converted as
    an experiment.

  - I handle test_expect_success and test_expect_failure here. If we
    like this style, we could easily extend it to other spots (e.g.,
    lazy_prereq bodies) on top of this patch.

  - even though we are using "local", we have to be careful about our
    variable names. Within test_expect_success, any variable we declare
    with local will be seen as local by the test snippets themselves (so
    it wouldn't persist between tests like normal variables would).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:23 -07:00
Jeff King
0c7d630220 chainlint.pl: add tests for test body in heredoc
The chainlint.pl script recently learned about the upcoming:

  test_expect_success 'some test' - <<\EOT
	TEST_BODY
  EOT

syntax, where TEST_BODY should be checked in the usual way. Let's make
sure this works by adding a few tests. The "here-doc-body" file tests
the basic syntax, including an embedded here-doc which we should still
be able to recognize.

Likewise the "here-doc-body-indent" checks the same thing, but using the
"<<-" operator. We wouldn't expect this to be used normally, but we
would not want to accidentally miss a body that uses it. The
"pathological" variant checks the opposite: we don't get confused by an
indented tag within the here-doc body.

The "here-doc-double" tests the handling of two here-doc tags on the
same line. This is not something we'd expect anybody to do in practice,
but the code was written defensively to handle this, so let's make sure
it works.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:22 -07:00
Eric Sunshine
a4a5f282f5 chainlint.pl: recognize test bodies defined via heredoc
In order to check tests for semantic problems, chainlint.pl scans test
scripts, looking for tests defined as:

    test_expect_success [prereq] title '
        body
    '

where `body` is a single string which is then treated as a standalone
chunk of code and "linted" to detect semantic issues. (The same happens
for `test_expect_failure` definitions.)

The introduction of test definitions in which the test body is instead
presented via a heredoc rather than as a single string creates a blind
spot in the linting process since such invocations are not recognized by
chainlint.pl.

Prepare for this new style by also recognizing tests defined as:

    test_expect_success [prereq] title - <<\EOT
        body
    EOT

A minor complication is that chainlint.pl has never considered heredoc
bodies significant since it doesn't scan them for semantic problems,
thus it has always simply thrown them away. However, with the new
`test_expect_success` calling sequence, heredoc bodies become
meaningful, thus need to be captured.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:22 -07:00
Jeff King
03763e68fb chainlint.pl: check line numbers in expected output
While working on chainlint.pl recently, we introduced some bugs that
showed incorrect line numbers in the output. But it was hard to notice,
since we sanitize the output by removing all of the line numbers! It
would be nice to retain these so we can catch any regressions.

The main reason we sanitize is for maintainability: we concatenate all
of the test snippets into a single file, so it's hard for each ".expect"
file to know at which offset its test input will be found. We can handle
that by storing the per-test line numbers in the ".expect" files, and
then dynamically offsetting them as we build the concatenated test and
expect files together.

The changes to the ".expect" files look like tedious boilerplate, but it
actually makes adding new tests easier. You can now just run:

  perl chainlint.pl chainlint/foo.test |
  tail -n +2 >chainlint/foo.expect

to save the output of the script minus the comment headers (after
checking that it is correct, of course). Whereas before you had to strip
the line numbers. The conversions here were done mechanically using
something like the script above, and then spot-checked manually.

It would be possible to do all of this in shell via the Makefile, but it
gets a bit complicated (and requires a lot of extra processes). Instead,
I've written a short perl script that generates the concatenated files
(we already depend on perl, since chainlint.pl uses it). Incidentally,
this improves a few other things:

  - we incorrectly used $(CHAINLINTTMP_SQ) inside a double-quoted
    string. So if your test directory required quoting, like:

       make "TEST_OUTPUT_DIRECTORY=/tmp/h'orrible"

    we'd fail the chainlint tests.

  - the shell in the Makefile didn't handle &&-chaining correctly in its
    loops (though in practice the "sed" and "cat" invocations are not
    likely to fail).

  - likewise, the sed invocation to strip numbers was hiding the exit
    code of chainlint.pl itself. In practice this isn't a big deal;
    since there are linter violations in the test files, we expect it to
    exit non-zero. But we could later use exit codes to distinguish
    serious errors from expected ones.

  - we now use a constant number of processes, instead of scaling with
    the number of test scripts. So it should be a little faster (on my
    machine, "make check-chainlint" goes from 133ms to 73ms).

There are some alternatives to this approach, but I think this is still
a good intermediate step:

  1. We could invoke chainlint.pl individually on each test file, and
     compare it to the expected output (and possibly using "make" to
     avoid repeating already-done checks). This is a much bigger change
     (and we'd have to figure out what to do with the "# LINT" lines in
     the inputs). But in this case we'd still want the "expect" files to
     be annotated with line numbers. So most of what's in this patch
     would be needed anyway.

  2. Likewise, we could run a single chainlint.pl and feed it all of the
     scripts (with "--jobs=1" to get deterministic output). But we'd
     still need to annotate the scripts as we did here, and we'd still
     need to either assemble the "expect" file, or break apart the
     script output to compare to each individual ".expect" file.

So we may pursue those in the long run, but this patch gives us more
robust tests without too much extra work or moving in a useless
direction.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:22 -07:00
Jeff King
382f6edaee chainlint.pl: force CRLF conversion when opening input files
The lexer in chainlint.pl can't handle CRLF line endings; it complains
about an internal error in scan_token() if we see one. For example, in
our Windows CI environment:

  $ perl chainlint.pl chainlint/for-loop.test | cat -v
  Thread 2 terminated abnormally: internal error scanning character '^M'

This doesn't break "make check-chainlint" (yet), because we assemble a
concatenated input by passing the contents of each file through "sed".
And the "sed" we use will strip out the CRLFs. But the next patch is
going to rework this a bit, which does break check-chainlint on Windows.
Plus it's probably nicer to folks on Windows who might work on chainlint
itself and write new tests.

In theory we could fix the parser to handle this, but it's not really
worth the trouble. We should be able to ask the input layer to translate
the line endings for us. In fact, I'd expect this to happen by default,
as perl's documentation claims Win32 uses the ":unix:crlf" PERLIO layer
by default ("unix" here just refers to using read/write syscalls, and
then "crlf" layers the translation on top). However, this doesn't seem
to be the case in our Windows CI environment. I didn't dig into the
exact reason, but it is perhaps because we are using an msys build of
perl rather than a "true" Win32 build.

At any rate, it is easy-ish to just ask explicitly for the conversion.
In the above example, setting PERLIO=crlf in the environment is enough
to make it work. Curiously, though, this doesn't work when invoking
chainlint via "make". Again, I didn't dig into it, but it may have to do
with msys programs calling Windows programs or vice versa.

We can make it work consistently by just explicitly asking for CRLF
translation when we open the files. This will even work on non-Windows
platforms, though we wouldn't really expect to find CRLF files there.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:22 -07:00
Jeff King
d558509e25 chainlint.pl: do not spawn more threads than we have scripts
The chainlint.pl script spawns worker threads to check many scripts in
parallel. This is good if you feed it a lot of scripts. But if you give
it few (or one), then the overhead of spawning the threads dominates. We
can easily notice that we have fewer scripts than threads and scale back
as appropriate.

This patch reduces the time to run:

  time for i in chainlint/*.test; do
	perl chainlint.pl $i
  done >/dev/null

on my system from ~4.1s to ~1.1s, where I have 8+8 cores.

As with the previous patch, this isn't the usual way we run chainlint
(we feed many scripts at once, which is why it supports threading in the
first place). So this won't make a big difference in the real world, but
it may help us out in the future, and it makes experimenting with and
debugging the chainlint tests a bit more pleasant.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:21 -07:00
Jeff King
a7c1c10256 chainlint.pl: only start threads if jobs > 1
If the system supports threads, chainlint.pl will always spawn worker
threads to do the real work. But when --jobs=1, this is pointless, since
we could just do the work in the main thread. And spawning even a single
thread has a high overhead. For example, on my Linux system, running:

  for i in chainlint/*.test; do
	perl chainlint.pl --jobs=1 $i
  done >/dev/null

takes ~1.7s without this patch, and ~1.1s after. We don't usually spawn
a bunch of individual chainlint.pl processes (instead we feed several
scripts at once, and the parallelism outweighs the setup cost). But it's
something we've considered doing, and since we already have fallback
code for systems without thread support, it's pretty easy to make this
work.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:21 -07:00
Jeff King
a5e450144d chainlint.pl: add test_expect_success call to test snippets
The chainlint tests are a series of individual files, each holding a
test body. The "make check-chainlint" target assembles them into a
single file, adding a "test_expect_success" function call around each.
Let's instead include that function call in the files themselves. This
is a little more boilerplate, but has several advantages:

  1. You can now run chainlint manually on snippets with just "perl
     chainlint.perl chainlint/foo.test". This can make developing and
     debugging a little easier.

  2. Many of the tests implicitly relied on the syntax of the lines
     added by the Makefile (in particular the use of single-quotes).
     This assumption is much easier to see when the single-quotes are
     alongside the test body.

  3. We had no way to test how the chainlint program handled
     various test_expect_success lines themselves. Now we'll be able to
     check variations.

The change to the .test files was done mechanically, using the same
test names they would have been assigned by the Makefile (this is
important to match the expected output). The Makefile has the minimal
change to drop the extra lines; there are more cleanups possible but a
future patch in this series will rewrite this substantially anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-10 10:14:21 -07:00
Junio C Hamano
4f5822076f http.c: cookie file tightening
The http.cookiefile configuration variable is used to call
curl_easy_setopt() to set CURLOPT_COOKIEFILE and if http.savecookies
is set, the same value is used for CURLOPT_COOKIEJAR.  The former is
used only to read cookies at startup, the latter is used to write
cookies at the end.

The manual pages https://curl.se/libcurl/c/CURLOPT_COOKIEFILE.html
and https://curl.se/libcurl/c/CURLOPT_COOKIEJAR.html talk about two
interesting special values.

 * "" (an empty string) given to CURLOPT_COOKIEFILE means not to
   read cookies from any file upon startup.

 * It is not specified what "" (an empty string) given to
   CURLOPT_COOKIEJAR does; presumably open a file whose name is an
   empty string and write cookies to it?  In any case, that is not
   what we want to see happen, ever.

 * "-" (a dash) given to CURLOPT_COOKIEFILE makes cURL read cookies
   from the standard input, and given to CURLOPT_COOKIEJAR makes
   cURL write cookies to the standard output.  Neither of which we
   want ever to happen.

So, let's make sure we avoid these nonsense cases.  Specifically,
when http.cookies is set to "-", ignore it with a warning, and when
it is set to "" and http.savecookies is set, ignore http.savecookies
with a warning.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:28:38 -07:00
brian m. carlson
610cbc1dfb http: allow authenticating proactively
When making a request over HTTP(S), Git only sends authentication if it
receives a 401 response.  Thus, if a repository is open to the public
for reading, Git will typically never ask for authentication for fetches
and clones.

However, there may be times when a user would like to authenticate
nevertheless.  For example, a forge may give higher rate limits to users
who authenticate because they are easier to contact in case of excessive
use.  Or it may be useful for a known heavy user, such as an internal
service, to proactively authenticate so its use can be monitored and, if
necessary, throttled.

Let's make this possible with a new option, "http.proactiveAuth".  This
option specifies a type of authentication which can be used to
authenticate against the host in question.  This is necessary because we
lack the WWW-Authenticate header to provide us details; similarly, we
cannot accept certain types of authentication because we require
information from the server, such as a nonce or challenge, to
successfully authenticate.

If we're in auto mode and we got a username and password, set the
authentication scheme to Basic.  libcurl will not send authentication
proactively unless there's a single choice of allowed authentication,
and we know in this case we didn't get an authtype entry telling us what
scheme to use, or we would have taken a different codepath and written
the header ourselves.  In any event, of the other schemes that libcurl
supports, Digest and NTLM require a nonce or challenge, which means that
they cannot work with proactive auth, and GSSAPI does not use a username
and password at all, so Basic is the only logical choice among the
built-in options.

Note that the existing http_proactive_auth variable signifies proactive
auth if there are already credentials, which is different from the
functionality we're adding, which always seeks credentials even if none
are provided.  Nonetheless, t5540 tests the existing behavior for
WebDAV-based pushes to an open repository without credentials, so we
preserve it.  While at first this may seem an insecure and bizarre
decision, it may be that authentication is done with TLS certificates,
in which case it might actually provide a quite high level of security.
Expand the variable to use an enum to handle the additional cases and a
helper function to distinguish our new cases from the old ones.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:27:51 -07:00
brian m. carlson
70405acf60 doc: mention that proxies must be completely transparent
We already document in the FAQ that proxies must be completely
transparent and not modify the request or response in any way, but add
similar documentation to the http.proxy entry.  We know that while the
FAQ is very useful, users sometimes are less likely to read in favor of
the documentation specific to an option or command, so adding it in both
places will help users be adequately informed.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:24:42 -07:00
brian m. carlson
804ecbcfd1 gitfaq: add entry about syncing working trees
Users very commonly want to sync their working tree with uncommitted
changes across machines, often to carry across in-progress work or
stashes.  Despite this not being a recommended approach, users want to
do it and are not dissuaded by suggestions not to, so let's recommend a
sensible technique.

The technique that many users are using is their preferred cloud syncing
service, which is a bad idea.  Users have reported problems where they
end up with duplicate files that won't go away (with names like "file.c
2"), broken references, oddly named references that have date stamps
appended to them, missing objects, and general corruption and data loss.
That's because almost all of these tools sync file by file, which is a
great technique if your project is a single word processing document or
spreadsheet, but is utterly abysmal for Git repositories because they
don't necessarily snapshot the entire repository correctly.  They also
tend to sync the files immediately instead of when the repository is
quiescent, so writing multiple files, as occurs during a commit or a gc,
can confuse the tools and lead to corruption.

We know that the old standby, rsync, is up to the task, provided that
the repository is quiescent, so let's suggest that and dissuade people
from using cloud syncing tools.  Let's tell people about common things
they should be aware of before doing this and that this is still
potentially risky.  Additionally, let's tell people that Git's security
model does not permit sharing working trees across users in case they
planned to do that.  While we'd still prefer users didn't try to do
this, hopefully this will lead them in a safer direction.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:24:42 -07:00
brian m. carlson
c98f78b806 gitfaq: give advice on using eol attribute in gitattributes
In the FAQ, we tell people how to use the text attribute, but we fail to
explain what to do with the eol attribute.  As we ourselves have
noticed, most shell implementations do not care for carriage returns,
and as such, people will practically always want them to use LF endings.
Similar things can be said for batch files on Windows, except with CRLF
endings.

Since these are common things to have in a repository, let's help users
make a good decision by recommending that they use the gitattributes
file to correctly check out the endings.

In addition, let's correct the cross-reference to this question, which
originally referred to "the following entry", even though a new entry
has been inserted in between.  The cross-reference notation should
prevent this from occurring and provide a link in formats, such as HTML,
which support that.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:24:42 -07:00
brian m. carlson
2101341484 gitfaq: add documentation on proxies
Many corporate environments and local systems have proxies in use.  Note
the situations in which proxies can be used and how to configure them.
At the same time, note what standards a proxy must follow to work with
Git.  Explicitly call out certain classes that are known to routinely
have problems reported various places online, including in the Git for
Windows issue tracker and on Stack Overflow, and recommend against the
use of such software, noting that they are associated with myriad
security problems (including, for example, breaking sandboxing and image
integrity[0], and, for TLS middleboxes, the use of insecure protocols
and ciphers and lack of certificate verification[1]). Don't mention the
specific nature of these security problems in the FAQ entry because they
are extremely numerous and varied and we wish to keep the FAQ entry
relatively brief.

[0] https://issues.chromium.org/issues/40285192
[1] https://faculty.cc.gatech.edu/~mbailey/publications/ndss17_interception.pdf

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-07-09 21:24:42 -07:00