* [PATCH v4] fuzz: add new oss-fuzz fuzzer for date.c / date.h
From: Arthur Chan via GitGitGadget @ 2023-11-17 17:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Arthur Chan, Arthur Chan
In-Reply-To: <pull.1612.v3.git.1699959186146.gitgitgadget@gmail.com>
From: Arthur Chan <arthur.chan@adalogics.com>
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
---
fuzz: add new oss-fuzz fuzzer for date.c / date.h
This patch is aimed to add a new oss-fuzz fuzzer to the oss-fuzz
directory for fuzzing date.c / date.h in the base directory.
The .gitignore of the oss-fuzz directory and the Makefile have been
modified to accommodate the new fuzzer fuzz-date.c.
Fixed the objects order in .gitignore and Makefiles and fixed some of
the logic and formatting for the fuzz-date.c fuzzer in v2.
Fixed the creation and memory allocation of the fuzzing str in v3. Also
fixed the tz type and sign-extended the data before passing to the tz
variable.
Fixed the tz variable allocations and some of the bytes used for fuzzing
variables in v4.
Comment: Yes, indeed. It is quite annoying to have that twice. Yes, the
tz should be considered as attacker controllable and thus negative
values should be considered. But it is tricky to fuzz it because the
date.c::gm_time_t() will call die() if the value is invalid and that
exit the fuzzer directly. OSS-Fuzz may consider it as an issue (or bug)
because the fuzzer exit "unexpectedly". I agree that if we consider the
tz as "attacker controllable, we should include negative values, but
since it will cause the fuzzer exit, I am not sure if it is the right
approach from the fuzzing perspective. Also, it is something that date.c
already take care of with the conditional checking, thus it may also be
worth to do some checking and exclude some invalid values before calling
date.c::show_date() but this may result in copying some conditional
checking code from date.c.
Additional comment for v4: Thanks for the suggestion. Yes, that maybe
the easier approach. Since the new logic is only using 2 bytes for the
int16_t tz, thus the local and dmtype variable could use separate bytes
to increase "randomness".
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1612%2Farthurscchan%2Fnew-fuzzer-date-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1612/arthurscchan/new-fuzzer-date-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1612
Range-diff vs v3:
1: 046bca32889 ! 1: 33a72d4c197 fuzz: add new oss-fuzz fuzzer for date.c / date.h
@@ oss-fuzz/fuzz-date.c (new)
+{
+ int local;
+ int num;
-+ int tz;
+ char *str;
-+ int8_t *tmp_data;
++ int16_t tz;
+ timestamp_t ts;
+ enum date_mode_type dmtype;
+ struct date_mode *dm;
+
+ if (size <= 4)
+ /*
-+ * we use the first byte to fuzz dmtype and local,
-+ * then the next three bytes to fuzz tz offset,
-+ * and the remainder (at least one byte) is fed
-+ * as end-user input to approxidate_careful().
++ * we use the first byte to fuzz dmtype and the
++ * second byte to fuzz local, then the next two
++ * bytes to fuzz tz offset. The remainder
++ * (at least one byte) is fed as input to
++ * approxidate_careful().
+ */
+ return 0;
+
-+ local = !!(*data & 0x10);
-+ num = *data % DATE_UNIX;
++ local = !!(*data++ & 0x10);
++ num = *data++ % DATE_UNIX;
+ if (num >= DATE_STRFTIME)
+ num++;
+ dmtype = (enum date_mode_type)num;
-+ data++;
-+ size--;
++ size -= 2;
+
-+ tmp_data = (int8_t*)data;
-+ tz = *tmp_data++;
-+ tz = (tz << 8) | *tmp_data++;
-+ tz = (tz << 8) | *tmp_data++;
-+ data += 3;
-+ size -= 3;
++ tz = *data++;
++ tz = (tz << 8) | *data++;
++ size -= 2;
+
+ str = xmemdupz(data, size);
+
@@ oss-fuzz/fuzz-date.c (new)
+
+ dm = date_mode_from_type(dmtype);
+ dm->local = local;
-+ show_date(ts, tz, dm);
++ show_date(ts, (int)tz, dm);
+
+ date_mode_release(dm);
+
Makefile | 1 +
oss-fuzz/.gitignore | 1 +
oss-fuzz/fuzz-date.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+)
create mode 100644 oss-fuzz/fuzz-date.c
diff --git a/Makefile b/Makefile
index 03adcb5a480..4b875ef6ce1 100644
--- a/Makefile
+++ b/Makefile
@@ -750,6 +750,7 @@ SCRIPTS = $(SCRIPT_SH_GEN) \
ETAGS_TARGET = TAGS
FUZZ_OBJS += oss-fuzz/fuzz-commit-graph.o
+FUZZ_OBJS += oss-fuzz/fuzz-date.o
FUZZ_OBJS += oss-fuzz/fuzz-pack-headers.o
FUZZ_OBJS += oss-fuzz/fuzz-pack-idx.o
.PHONY: fuzz-objs
diff --git a/oss-fuzz/.gitignore b/oss-fuzz/.gitignore
index 9acb74412ef..5b954088254 100644
--- a/oss-fuzz/.gitignore
+++ b/oss-fuzz/.gitignore
@@ -1,3 +1,4 @@
fuzz-commit-graph
+fuzz-date
fuzz-pack-headers
fuzz-pack-idx
diff --git a/oss-fuzz/fuzz-date.c b/oss-fuzz/fuzz-date.c
new file mode 100644
index 00000000000..036378b946f
--- /dev/null
+++ b/oss-fuzz/fuzz-date.c
@@ -0,0 +1,49 @@
+#include "git-compat-util.h"
+#include "date.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+ int local;
+ int num;
+ char *str;
+ int16_t tz;
+ timestamp_t ts;
+ enum date_mode_type dmtype;
+ struct date_mode *dm;
+
+ if (size <= 4)
+ /*
+ * we use the first byte to fuzz dmtype and the
+ * second byte to fuzz local, then the next two
+ * bytes to fuzz tz offset. The remainder
+ * (at least one byte) is fed as input to
+ * approxidate_careful().
+ */
+ return 0;
+
+ local = !!(*data++ & 0x10);
+ num = *data++ % DATE_UNIX;
+ if (num >= DATE_STRFTIME)
+ num++;
+ dmtype = (enum date_mode_type)num;
+ size -= 2;
+
+ tz = *data++;
+ tz = (tz << 8) | *data++;
+ size -= 2;
+
+ str = xmemdupz(data, size);
+
+ ts = approxidate_careful(str, &num);
+ free(str);
+
+ dm = date_mode_from_type(dmtype);
+ dm->local = local;
+ show_date(ts, (int)tz, dm);
+
+ date_mode_release(dm);
+
+ return 0;
+}
base-commit: dadef801b365989099a9929e995589e455c51fed
--
gitgitgadget
^ permalink raw reply related
* Re: Migration of git-scm.com to a static web site: ready for review/testing
From: Todd Zullinger @ 2023-11-17 16:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Matt Burke, Victoria Dye, Matthias Aßhauer
In-Reply-To: <6f7d20b4-a725-0ef9-f6d3-ff2810da9e7a@gmx.de>
Hello,
Johannes Schindelin wrote:
> At this point, the patches are fairly robust and I am mainly hoping for
> help with verifying that the static site works as intended, that existing
> links will continue to work with the new site (essentially, find obscure
> references to the existing website, then insert `git.github.io/` in the
> URL and verify that it works as intended).
>
> To that end, I deployed this branch to GitHub Pages so that anyone
> interested (hopefully many!) can have a look at
> https://git.github.io/git-scm.com/ and compare to the existing
> https://git-scm.com/.
This is nice. Thanks to all for working on it!
For checking links, a tool like linkcheker[1] is very handy.
This is run against the local docs in the Fedora package
builds to catch broken links.
I ran it against the test site and it turned up _a lot_ of
broken links. It's enough that saving and sharing the
output is probably more work than having someone familiar
with the migration give it a run directly.
I ran `linkchecker https://git.github.io/git-scm.com/` and
the eventual result was:
That's it. 13459 links in 14126 URLs checked. 0 warnings found. 6763 errors found.
Stopped checking at 2023-11-17 11:11:17-004 (1 hour, 19 minutes)
The default output reports failures in a format like this:
URL `ch00/ch10-git-internals'
Name `Git Internals'
Parent URL https://git.github.io/git-scm.com/book/tr/v2/Ek-b%C3%B6l%C3%BCm-C:-Git-Commands-Plumbing-Commands/, line 106, col 1318
Real URL https://git.github.io/git-scm.com/book/tr/v2/Ek-b%C3%B6l%C3%BCm-C:-Git-Commands-Plumbing-Commands/ch00/ch10-git-internals
Check time 3.303 seconds
Size 1KB
Result Error: 404 Not Found
LinkChecker can be run in a mode which directs the failures
to a file. That would be more like:
linkchecker -F text/utf_8//tmp/git-scm-check.txt https://git.github.io/git-scm.com/
The format of the -F option is TYPE[/ENCODING][/FILENAME]
where TYPE can be text, html, sql, csv, gml, dot, xml,
sitemap, none or failures. The failures type is much more
terse:
1 "('https://git.github.io/git-scm.com/book/en/v2/Appendix-C:-Git-Commands-Plumbing-Commands/', 'https://git.github.io/git-scm.com/book/en/v2/Appendix-C:-Git-Commands-Plumbing-Commands/ch00/ch10-git-internals')"
I found the text type much more helpful in quickly spot
checking some of the failures since it includes the text
string used for the link.
Running it against a local directory of the content would be
a lot faster, if that's an option. It's also worth bumping
the default number of threads from 10 to increase the speed
a bit.
[1] https://linkchecker.github.io/linkchecker/
--
Todd
^ permalink raw reply
* Migration of git-scm.com to a static web site: ready for review/testing
From: Johannes Schindelin @ 2023-11-17 13:25 UTC (permalink / raw)
To: git; +Cc: Matt Burke, Victoria Dye, Matthias Aßhauer
[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]
Hi,
the idea of migrating https://git-scm.com/ from a Rails app to a static
site has been discussed several times on this list in the past.
Thanks to the heroic, multi-year efforts of Matt Burke, Victoria Dye and
Matthias Aßhauer, there is now a Pull Request, ready for review:
https://github.com/git/git-scm.com/pull/1804
This Pull Request is not for the faint of heart, mainly because of the
sheer amount of generated pages that are committed to the repository (such
as the book, the manual pages, etc, a design decision necessary to run
this as a static website).
These pages are generated by GitHub workflows that are intended to run on
a schedule, and the scripts that generate them are part of the Pull
Request. For that reason, I do not consider it necessary to review those
generated pages, those reviews have been done in the upstream sources from
which the pages were generated.
At this point, the patches are fairly robust and I am mainly hoping for
help with verifying that the static site works as intended, that existing
links will continue to work with the new site (essentially, find obscure
references to the existing website, then insert `git.github.io/` in the
URL and verify that it works as intended).
To that end, I deployed this branch to GitHub Pages so that anyone
interested (hopefully many!) can have a look at
https://git.github.io/git-scm.com/ and compare to the existing
https://git-scm.com/.
Thank you,
Johannes
^ permalink raw reply
* [PATCH] Fix a typo in `each_file_in_pack_dir_fn()`'s declaration
From: Johannes Schindelin via GitGitGadget @ 2023-11-17 13:15 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
One parameter is called `file_pach`. On the face of it, this looks as if
it was supposed to talk about a `path` instead of a `pach`.
However, looking at the way this callback is called, it gets fed the
`d_name` from a directory entry, which provides just the file name, not
the full path. Therefore, let's fix this by calling the parameter
`file_name` instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
packfile.h: fix a typo
I stumbled over this typo yesterday. Nothing about this patch is urgent,
of course, it can easily wait until v2.43.0 is released.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1614%2Fdscho%2Fpackfile.h-typo-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1614/dscho/packfile.h-typo-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1614
packfile.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/packfile.h b/packfile.h
index c3692308b8d..28c8fd3e39a 100644
--- a/packfile.h
+++ b/packfile.h
@@ -54,7 +54,7 @@ const char *pack_basename(struct packed_git *p);
struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
- const char *file_pach, void *data);
+ const char *file_name, void *data);
void for_each_file_in_pack_dir(const char *objdir,
each_file_in_pack_dir_fn fn,
void *data);
base-commit: cfb8a6e9a93adbe81efca66e6110c9b4d2e57169
--
gitgitgadget
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2023, #07; Fri, 17)
From: Kristoffer Haugsbakk @ 2023-11-17 10:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqpm08pyrh.fsf@gitster.g>
On Fri, Nov 17, 2023, at 09:30, Junio C Hamano wrote:
> * kh/t7900-cleanup (2023-10-17) 9 commits
> - t7900: fix register dependency
> - t7900: factor out packfile dependency
> - t7900: fix `print-args` dependency
> - t7900: fix `pfx` dependency
> - t7900: factor out common schedule setup
> - t7900: factor out inheritance test dependency
> - t7900: create commit so that branch is born
> - t7900: setup and tear down clones
> - t7900: remove register dependency
>
> Test clean-up.
>
> Perhaps discard?
> cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
> source: <cover.1697319294.git.code@khaugsbakk.name>
There has been no interest in it so I say yes to discarding.
^ permalink raw reply
* What's cooking in git.git (Nov 2023, #07; Fri, 17)
From: Junio C Hamano @ 2023-11-17 8:30 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Git 2.43-rc2 has been tagged.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[New Topics]
* jw/builtin-objectmode-attr (2023-11-16) 2 commits
- SQUASH???
- attr: add builtin objectmode values support
The builtin_objectmode attribute is populated for each path
without adding anything in .gitattributes files, which would be
useful in magic pathspec, e.g., ":(attr:builtin_objectmode=100755)"
to limit to executables.
source: <20231116054437.2343549-1-jojwang@google.com>
* ps/ref-deletion-updates (2023-11-17) 4 commits
- refs: remove `delete_refs` callback from backends
- refs: deduplicate code to delete references
- refs/files: use transactions to delete references
- t5510: ensure that the packed-refs file needs locking
Simplify API implementation to delete references by eliminating
duplication.
Will merge to 'next'.
source: <cover.1699951815.git.ps@pks.im>
* tz/send-email-helpfix (2023-11-16) 1 commit
(merged to 'next' on 2023-11-17 at 8422271795)
+ send-email: remove stray characters from usage
Typoes in "git send-email -h" have been corrected.
Will merge to 'master'.
source: <20231115173952.339303-3-tmz@pobox.com>
* tz/send-email-negatable-options (2023-11-17) 2 commits
(merged to 'next' on 2023-11-17 at f09e533e43)
+ send-email: avoid duplicate specification warnings
+ perl: bump the required Perl version to 5.8.1 from 5.8.0
Newer versions of Getopt::Long started giving warnings against our
(ab)use of it in "git send-email". Bump the minimum version
requirement for Perl to 5.8.1 (from September 2002) to allow
simplifying our implementation.
Will cook in 'next'.
source: <20231116193014.470420-1-tmz@pobox.com>
--------------------------------------------------
[Stalled]
* pw/rebase-sigint (2023-09-07) 1 commit
- rebase -i: ignore signals when forking subprocesses
If the commit log editor or other external programs (spawned via
"exec" insn in the todo list) receive internactive signal during
"git rebase -i", it caused not just the spawned program but the
"Git" process that spawned them, which is often not what the end
user intended. "git" learned to ignore SIGINT and SIGQUIT while
waiting for these subprocesses.
Expecting a reroll.
cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>
* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
- cherry-pick: refuse cherry-pick sequence if index is dirty
"git cherry-pick A" that replays a single commit stopped before
clobbering local modification, but "git cherry-pick A..B" did not,
which has been corrected.
Expecting a reroll.
cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>
* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
- diff-lib: fix check_removed() when fsmonitor is active
- Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
- Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
(this branch uses jc/fake-lstat.)
The optimization based on fsmonitor in the "diff --cached"
codepath is resurrected with the "fake-lstat" introduced earlier.
It is unknown if the optimization is worth resurrecting, but in case...
source: <xmqqr0n0h0tw.fsf@gitster.g>
--------------------------------------------------
[Cooking]
* js/ci-discard-prove-state (2023-11-14) 1 commit
(merged to 'next' on 2023-11-14 at fade3ba143)
+ ci: avoid running the test suite _twice_
(this branch uses ps/ci-gitlab.)
The way CI testing used "prove" could lead to running the test
suite twice needlessly, which has been corrected.
Will cook in 'next'.
source: <pull.1613.git.1699894837844.gitgitgadget@gmail.com>
* jk/chunk-bounds-more (2023-11-09) 9 commits
(merged to 'next' on 2023-11-13 at 3df4b18bea)
+ commit-graph: mark chunk error messages for translation
+ commit-graph: drop verify_commit_graph_lite()
+ commit-graph: check order while reading fanout chunk
+ commit-graph: use fanout value for graph size
+ commit-graph: abort as soon as we see a bogus chunk
+ commit-graph: clarify missing-chunk error messages
+ commit-graph: drop redundant call to "lite" verification
+ midx: check consistency of fanout table
+ commit-graph: handle overflow in chunk_size checks
(this branch is used by tb/pair-chunk-expect.)
Code clean-up for jk/chunk-bounds topic.
Will cook in 'next'.
source: <20231109070310.GA2697602@coredump.intra.peff.net>
* ps/httpd-tests-on-nixos (2023-11-11) 3 commits
(merged to 'next' on 2023-11-13 at 81bd6f5334)
+ t9164: fix inability to find basename(1) in Subversion hooks
+ t/lib-httpd: stop using legacy crypt(3) for authentication
+ t/lib-httpd: dynamically detect httpd and modules path
Portability tweak.
Will cook in 'next'.
source: <cover.1699596457.git.ps@pks.im>
* ss/format-patch-use-encode-headers-for-cover-letter (2023-11-10) 1 commit
(merged to 'next' on 2023-11-14 at 1a4bd59e15)
+ format-patch: fix ignored encode_email_headers for cover letter
"git format-patch --encode-email-headers" ignored the option when
preparing the cover letter, which has been corrected.
Will cook in 'next'.
source: <20231109111950.387219-1-contact@emersion.fr>
* ps/ban-a-or-o-operator-with-test (2023-11-11) 4 commits
(merged to 'next' on 2023-11-14 at d84471baab)
+ Makefile: stop using `test -o` when unlinking duplicate executables
+ contrib/subtree: convert subtree type check to use case statement
+ contrib/subtree: stop using `-o` to test for number of args
+ global: convert trivial usages of `test <expr> -a/-o <expr>`
Test and shell scripts clean-up.
Will cook in 'next'.
source: <cover.1699609940.git.ps@pks.im>
* vd/glossary-dereference-peel (2023-11-14) 1 commit
(merged to 'next' on 2023-11-17 at bac3ab0c0b)
+ glossary: add definitions for dereference & peel
"To dereference" and "to peel" were sometimes used in in-code
comments and documentation but without description in the glossary.
Will merge to 'master'.
source: <pull.1610.v2.git.1699917471769.gitgitgadget@gmail.com>
* ak/rebase-autosquash (2023-11-16) 3 commits
(merged to 'next' on 2023-11-17 at 3ed6e79445)
+ rebase: rewrite --(no-)autosquash documentation
+ rebase: support --autosquash without -i
+ rebase: fully ignore rebase.autoSquash without -i
"git rebase --autosquash" is now enabled for non-interactive rebase,
but it is still incompatible with the apply backend.
Will cook in 'next'.
source: <20231114214339.10925-1-andy.koppe@gmail.com>
* vd/for-each-ref-unsorted-optimization (2023-11-16) 10 commits
(merged to 'next' on 2023-11-17 at ff99420bf6)
+ t/perf: add perf tests for for-each-ref
+ ref-filter.c: use peeled tag for '*' format fields
+ for-each-ref: clean up documentation of --format
+ ref-filter.c: filter & format refs in the same callback
+ ref-filter.c: refactor to create common helper functions
+ ref-filter.c: rename 'ref_filter_handler()' to 'filter_one()'
+ ref-filter.h: add functions for filter/format & format-only
+ ref-filter.h: move contains caches into filter
+ ref-filter.h: add max_count and omit_empty to ref_format
+ ref-filter.c: really don't sort when using --no-sort
"git for-each-ref --no-sort" still sorted the refs alphabetically
which paid non-trivial cost. It has been redefined to show output
in an unspecified order, to allow certain optimizations to take
advantage of.
Will cook in 'next'.
source: <pull.1609.v2.git.1699991638.gitgitgadget@gmail.com>
* jw/git-add-attr-pathspec (2023-11-04) 1 commit
(merged to 'next' on 2023-11-13 at b61be94e4d)
+ attr: enable attr pathspec magic for git-add and git-stash
"git add" and "git stash" learned to support the ":(attr:...)"
magic pathspec.
Will cook in 'next'.
source: <20231103163449.1578841-1-jojwang@google.com>
* ps/ci-gitlab (2023-11-09) 8 commits
(merged to 'next' on 2023-11-10 at ea7ed67945)
+ ci: add support for GitLab CI
+ ci: install test dependencies for linux-musl
+ ci: squelch warnings when testing with unusable Git repo
+ ci: unify setup of some environment variables
+ ci: split out logic to set up failed test artifacts
+ ci: group installation of Docker dependencies
+ ci: make grouping setup more generic
+ ci: reorder definitions for grouping functions
(this branch is used by js/ci-discard-prove-state.)
Add support for GitLab CI.
Will cook in 'next'.
source: <cover.1699514143.git.ps@pks.im>
* ps/ref-tests-update (2023-11-03) 10 commits
(merged to 'next' on 2023-11-13 at dc26e55d6f)
+ t: mark several tests that assume the files backend with REFFILES
+ t7900: assert the absence of refs via git-for-each-ref(1)
+ t7300: assert exact states of repo
+ t4207: delete replace references via git-update-ref(1)
+ t1450: convert tests to remove worktrees via git-worktree(1)
+ t: convert tests to not access reflog via the filesystem
+ t: convert tests to not access symrefs via the filesystem
+ t: convert tests to not write references via the filesystem
+ t: allow skipping expected object ID in `ref-store update-ref`
+ Merge branch 'ps/show-ref' into ps/ref-tests-update
Update ref-related tests.
Will cook in 'next'.
source: <cover.1698914571.git.ps@pks.im>
* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
- fetch: no redundant error message for atomic fetch
- t5574: test porcelain output of atomic fetch
"git fetch --atomic" issued an unnecessary empty error message,
which has been corrected.
Expecting an update.
cf. <ZTjQIrCgSANAT8wR@tanuki>
source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>
* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
- bugreport: include +i in outfile suffix as needed
Instead of auto-generating a filename that is already in use for
output and fail the command, `git bugreport` learned to fuzz the
filename to avoid collisions with existing files.
Expecting a reroll.
cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
source: <20231016214045.146862-2-jacob@initialcommit.io>
* kh/t7900-cleanup (2023-10-17) 9 commits
- t7900: fix register dependency
- t7900: factor out packfile dependency
- t7900: fix `print-args` dependency
- t7900: fix `pfx` dependency
- t7900: factor out common schedule setup
- t7900: factor out inheritance test dependency
- t7900: create commit so that branch is born
- t7900: setup and tear down clones
- t7900: remove register dependency
Test clean-up.
Perhaps discard?
cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
source: <cover.1697319294.git.code@khaugsbakk.name>
* tb/merge-tree-write-pack (2023-10-23) 5 commits
- builtin/merge-tree.c: implement support for `--write-pack`
- bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
- bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- bulk-checkin: extract abstract `bulk_checkin_source`
"git merge-tree" learned "--write-pack" to record its result
without creating loose objects.
Broken when an object created during a merge is needed to continue merge
cf. <CABPp-BEfy9VOvimP9==ry_rZXu=metOQ8s=_-XiG_Pdx9c06Ww@mail.gmail.com>
source: <cover.1698101088.git.me@ttaylorr.com>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
(this branch uses jk/chunk-bounds-more.)
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2023-10-18) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new filter ver. that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Needs (hopefully final and quick) review.
source: <cover.1697653929.git.me@ttaylorr.com>
* cc/git-replay (2023-11-16) 14 commits
- replay: stop assuming replayed branches do not diverge
- replay: add --contained to rebase contained branches
- replay: add --advance or 'cherry-pick' mode
- replay: use standard revision ranges
- replay: make it a minimal server side command
- replay: remove HEAD related sanity check
- replay: remove progress and info output
- replay: add an important FIXME comment about gpg signing
- replay: change rev walking options
- replay: introduce pick_regular_commit()
- replay: die() instead of failing assert()
- replay: start using parse_options API
- replay: introduce new builtin
- t6429: remove switching aspects of fast-rebase
Introduce "git replay", a tool meant on the server side without
working tree to recreate a history.
source: <20231115143327.2441397-1-christian.couder@gmail.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
- doc: refer to internet archive
- doc: update links for andre-simon.de
- doc: update links to current pages
- doc: switch links to https
Stale URLs have been updated to their current counterparts (or
archive.org) and HTTP links are replaced with working HTTPS links.
Needs review.
source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>
* la/trailer-cleanups (2023-10-20) 3 commits
- trailer: use offsets for trailer_start/trailer_end
- trailer: find the end of the log message
- commit: ignore_non_trailer computes number of bytes to ignore
Code clean-up.
Comments?
source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
- archive: support remote archive from stateless transport
- transport-helper: call do_take_over() in connect_helper
- transport-helper: call do_take_over() in process_connect
- transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Needs review.
source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>
* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
- pkt-line: do not chomp newlines for sideband messages
- pkt-line: memorize sideband fragment in reader
- test-pkt-line: add option parser for unpack-sideband
Sideband demultiplexer fixes.
Needs review.
source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>
* js/config-parse (2023-09-21) 5 commits
- config-parse: split library out of config.[c|h]
- config.c: accept config_parse_options in git_config_from_stdin
- config: report config parse errors using cb
- config: split do_event() into start and flush operations
- config: split out config_parse_options
The parsing routines for the configuration files have been split
into a separate file.
Needs review.
source: <cover.1695330852.git.steadmon@google.com>
* jc/fake-lstat (2023-09-15) 1 commit
- cache: add fake_lstat()
(this branch is used by jc/diff-cached-fsmonitor-fix.)
A new helper to let us pretend that we called lstat() when we know
our cache_entry is up-to-date via fsmonitor.
Needs review.
source: <xmqqcyykig1l.fsf@gitster.g>
* js/doc-unit-tests (2023-11-10) 3 commits
(merged to 'next' on 2023-11-10 at 7d00ffd06b)
+ ci: run unit tests in CI
+ unit tests: add TAP unit test framework
+ unit tests: add a project plan document
(this branch is used by js/doc-unit-tests-with-cmake.)
Process to add some form of low-level unit tests has started.
Will cook in 'next'.
source: <cover.1699555664.git.steadmon@google.com>
* js/doc-unit-tests-with-cmake (2023-11-10) 7 commits
(merged to 'next' on 2023-11-10 at b4503c9c8c)
+ cmake: handle also unit tests
+ cmake: use test names instead of full paths
+ cmake: fix typo in variable name
+ artifacts-tar: when including `.dll` files, don't forget the unit-tests
+ unit-tests: do show relative file paths
+ unit-tests: do not mistake `.pdb` files for being executable
+ cmake: also build unit tests
(this branch uses js/doc-unit-tests.)
Update the base topic to work with CMake builds.
Will cook in 'next'.
source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
* rj/status-bisect-while-rebase (2023-10-16) 1 commit
- status: fix branch shown when not only bisecting
"git status" is taught to show both the branch being bisected and
being rebased when both are in effect at the same time.
Needs review.
source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>
--------------------------------------------------
[Discarded]
* jc/strbuf-comment-line-char (2023-11-01) 4 commits
. strbuf: move env-using functions to environment.c
. strbuf: make add_lines() public
. strbuf_add_commented_lines(): drop the comment_line_char parameter
. strbuf_commented_addf(): drop the comment_line_char parameter
Code simplification that goes directly against a past libification
topic. It is hard to judge because the "libification" is done
piecewise without seemingly clear design principle.
Will discard.
source: <cover.1698791220.git.jonathantanmy@google.com>
^ permalink raw reply
* Re: [PATCH v3 0/2] send-email: avoid duplicate specification warnings
From: Junio C Hamano @ 2023-11-16 22:32 UTC (permalink / raw)
To: Todd Zullinger
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231116193014.470420-1-tmz@pobox.com>
Todd Zullinger <tmz@pobox.com> writes:
>> This sounds like something that is worth describing in the log
>> message (and Release Notes).
>
> I think the new commit messages describe the changes better. I didn't
> include anything in RelNotes as I was presuming we'd leave this for
> 2.44 rather than risk causing any problems this late in the 2.43 cycle.
> If you think the risk is low and/or the benefit is high, I can add it to
> the 2.43.0 RelNotes.
Please don't worry about the release notes, which I'll do only when
the topic hits the 'master' branch. It was meant primarily as a
note to myself. And I agree that this would be material for 2.44
and later.
Both patches, plus the stray single quote fix patch, looked good.
Thanks.
^ permalink raw reply
* Re: gitmodulesSymlink .gitmodules is a symbolic link
From: Jeff King @ 2023-11-16 20:27 UTC (permalink / raw)
To: flobee.code; +Cc: git
In-Reply-To: <CAM30mqasEjEBbn1vSUwvKP6LLjAFSw3xoeLrB1zLWnH37Z2fUg@mail.gmail.com>
On Wed, Nov 15, 2023 at 05:01:10PM +0100, flobee.code wrote:
> But in general I think the exclusion of symlinks to git system files
> is a mistake. It is implemented too sweepingly in my eyes.
I agree that the fact that we reject these at such a low level makes it
hard to use the tooling to rewrite the history to fix it.
But because of the security implications of out-of-tree symlinks from
untrusted repositories, it's important to catch these consistently. I
see your use case is for in-tree links, but detecting that makes the
checks much more complex. It also has always been the case that symlinks
do not behave consistently, as Git does not follow them when reading
in-index versions of meta files.
> And `git` itself also aborts. So I can't solve the problems this way.
>
> git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
> Rewrite [SomeHash] (3/185) (0 seconds passed, remaining 0 predicted) \
> error: Invalid path '.gitmodules' Could not initialize the index
I didn't test, but you could probably get by with using "git replace" to
first fix up the offending trees, and then run filter-branch (though
there may be a lot of such trees, so you'd probably to script that
step). I also suspect that filter-repo would handle this better:
https://github.com/newren/git-filter-repo
but didn't try it myself.
-Peff
^ permalink raw reply
* Re: tb/merge-tree-write-pack, was Re: What's cooking in git.git (Nov 2023, #04; Thu, 9)
From: Jeff King @ 2023-11-16 20:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Taylor Blau, git
In-Reply-To: <14bdff31-5229-bbd5-3715-f77f52b832d5@gmx.de>
On Wed, Nov 15, 2023 at 01:57:00PM +0100, Johannes Schindelin wrote:
> - The scenario I want to address (and that I assumed the patch series
> under discussion tried to address, too) is a very specific, server-side
> scenario where many `merge-tree`/`replay` runs produce _many_ loose
> objects. Quite a fraction of those are produced by processes that run
> into a SIGTERM-enforced timeout, and the `tmp_objdir` approach would
> naturally help: unneeded loose objects would not even be added to the
> primary object database but be reaped with the temporary object
> databases.
Thanks, this paragraph helped me to understand why you are interested in
tmp_objdir in the first place (as opposed to just doing a gc-auto repack
after finishing the merge).
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/2] perl: bump the required Perl version to 5.8.1 from 5.8.0
From: Jeff King @ 2023-11-16 20:16 UTC (permalink / raw)
To: Todd Zullinger
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231116193014.470420-2-tmz@pobox.com>
On Thu, Nov 16, 2023 at 02:30:10PM -0500, Todd Zullinger wrote:
> The following commit will make use of a Getopt::Long feature which is
> only present in Perl >= 5.8.1. Document that as the minimum version we
> support.
>
> Many of our Perl scripts will continue to run with 5.8.0 but this change
> allows us to adjust them as needed without breaking any promises to our
> users.
>
> The Perl requirement was last changed in d48b284183 (perl: bump the
> required Perl version to 5.8 from 5.6.[21], 2010-09-24). At that time,
> 5.8.0 was 8 years old. It is now over 21 years old.
Thanks, IMHO this is long overdue. You mentioned 5.10 elsewhere in the
thread, and it came up recently in a discussion (it would allow the use
of "//" for defined-or). So we could perhaps go a bit farther. But I am
also fine with 5.8.1 for now, if that is all it takes for this fix (and
I expect the chance that it causes a problem for anybody to be close to
zero).
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
> I debated changing all the 'use 5.008;' lines here, as most don't
> actually require a newer Perl, but the previous bump did the same.
>
> I can see the merit in either direction.
>
> Changing it allows future contributors to be confident in relying on
> 5.8.1 features.
>
> Not changing it allows anyone stuck on 5.8.0 to continue using the perl
> scripts which don't actually require 5.8.1.
Yeah, I can see both sides of the argument. I think I'd err on the side
of bumping (as you did here). That lets somebody who will be affected
know immediately, rather than only finding out when we randomly depend
on a feature later.
All of this discussion could likewise go in the commit message. :)
> Tangentially, the Perl docs for 'use' function recommend against the
> 5.008001 form[1]:
>
> Specifying VERSION as a numeric argument of the form 5.024001 should
> generally be avoided as older less readable syntax compared to
> v5.24.1. Before perl 5.8.0 released in 2002 the more verbose numeric
> form was the only supported syntax, which is why you might see it in
> older code.
>
> use v5.24.1; # compile time version check
> use 5.24.1; # ditto
> use 5.024_001; # ditto; older syntax compatible with perl 5.6
>
> I'm not enough of a Perl coder to have a strong preference or desire to
> push for such a change, but I thought it was worth mentioning in case
> others wonder why we're using the 5.008001 form.
I doubt it matters too much either way. I suspect at the time we moved
to v5.8 the nicer syntax was still pretty new (having only been
introduced by v5.6, which we were moving off of) and that older versions
of perl might not give as nice a message when they see it. But given
that 5.6 is now 23 years old, we can probably assume nobody will use it
(or at least they will be accustomed to whatever ugly message it
produces).
But IMHO that should be done as a separate patch anyway.
-Peff
^ permalink raw reply
* Re: [PATCH] ci: avoid running the test suite _twice_
From: Jeff King @ 2023-11-16 20:02 UTC (permalink / raw)
To: Josh Steadmon
Cc: Johannes Schindelin via GitGitGadget, Phillip Wood, git,
Johannes Schindelin
In-Reply-To: <ZVU4EVcj0MDrSNcG@google.com>
On Wed, Nov 15, 2023 at 01:28:49PM -0800, Josh Steadmon wrote:
> The first part is easy, but I don't see a good way to get both shell
> tests and unit tests executing under the same `prove` process. For shell
> tests, we pass `--exec '$(TEST_SHELL_PATH_SQ)'` to prove, meaning that
> we use the specified shell as an interpreter for the test files. That
> will not work for unit test executables.
Yes, it's unfortunate that you can't set the "exec" flag per-script
(especially because without --exec it will auto-detect the right thing,
but then of course it won't use TEST_SHELL_PATH). But we can intercept
and do it ourselves, like:
diff --git a/t/Makefile b/t/Makefile
index 225aaf78ed..0b7c028eea 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -61,7 +61,7 @@ failed:
test -z "$$failed" || $(MAKE) $$failed
prove: pre-clean check-chainlint $(TEST_LINT)
- @echo "*** prove ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+ @echo "*** prove ***"; TEST_SHELL_PATH='$(TEST_SHELL_PATH_SQ)' $(CHAINLINTSUPPRESS) $(PROVE) --exec ./run-test.sh $(GIT_PROVE_OPTS) $(T) $(UNIT_TESTS) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
$(T):
diff --git a/t/run-test.sh b/t/run-test.sh
new file mode 100755
index 0000000000..69944029c8
--- /dev/null
+++ b/t/run-test.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+case "$1" in
+*.sh)
+ exec ${TEST_SHELL_PATH:-/bin/sh} "$@"
+ ;;
+*)
+ exec "$@"
+ ;;
+esac
You can actually do this inside the prove script using their plugin
interface, but the necessary bits are somewhat arcane.
> We could bundle all the unit tests into a single shell script, but then
> we lose parallelization and add hoops to jump through to determine what
> breaks. Or we could autogenerate a corresponding shell script to run
> each individual unit test, but that seems gross. Of course, these are
> hypothetical concerns for now, since we only have a single unit test at
> the moment.
We can't just stick them all in a single script; there must be exactly
one "plan" line in the TAP output from a given source. I had imagined
just manually adding a thin wrapper for each ("t9970-unit-strbuf" or
something). But it would also be easy to autogenerate them while
compiling. (Although all of that is moot with the wrapper I showed
above).
> There's also the issue that the shell test arguments we pass on from
> prove would be shared with the unit tests. That's fine for now, as
> t-strbuf doesn't accept any runtime arguments, but it's possible that
> either the framework or individual unit tests might grow to need
> arguments, and it might not be convenient to stay compatible with the
> shell tests.
Sharing the options between the two seems like a benefit to me. I'd
think that "-v" and "-i" would be useful, at least. Options which don't
apply (e.g., "--root") could be quietly ignored.
-Peff
^ permalink raw reply related
* [PATCH] send-email: remove stray characters from usage
From: Todd Zullinger @ 2023-11-16 19:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqzfzes37f.fsf@gitster.g>
A few stray single quotes crept into the usage string in a2ce608244
(send-email docs: add format-patch options, 2021-10-25). Remove them.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
[I trimmed the Cc: list]
Junio C Hamano wrote:
> Thanks. Let's split this out as a docfix patch and handle it
> separately.
Done. :)
git-send-email.perl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index cacdbd6bb2..d24e981d61 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,8 +28,8 @@
sub usage {
print <<EOT;
-git send-email' [<options>] <file|directory>
-git send-email' [<options>] <format-patch options>
+git send-email [<options>] <file|directory>
+git send-email [<options>] <format-patch options>
git send-email --dump-aliases
Composing:
--
2.43.0.rc2
^ permalink raw reply related
* [PATCH v3 2/2] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-16 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231116193014.470420-1-tmz@pobox.com>
A warning is issued for options which are specified more than once
beginning with perl-Getopt-Long >= 2.55. In addition to causing users
to see warnings, this results in test failures which compare the output.
An example, from t9001-send-email.37:
| +++ diff -u expect actual
| --- expect 2023-11-14 10:38:23.854346488 +0000
| +++ actual 2023-11-14 10:38:23.848346466 +0000
| @@ -1,2 +1,7 @@
| +Duplicate specification "no-chain-reply-to" for option "no-chain-reply-to"
| +Duplicate specification "to-cover|to-cover!" for option "to-cover"
| +Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
| +Duplicate specification "no-thread" for option "no-thread"
| +Duplicate specification "no-to-cover" for option "no-to-cover"
| fatal: longline.patch:35 is longer than 998 characters
| warning: no patches were sent
| error: last command exited with $?=1
| not ok 37 - reject long lines
Remove the duplicate option specs. These are primarily the explicit
'--no-' prefix opts which were added in f471494303 (git-send-email.perl:
support no- prefix with older GetOptions, 2015-01-30). This was done
specifically to support perl-5.8.0 which includes Getopt::Long 2.32[1].
Getopt::Long 2.33 added support for the '--no-' prefix natively by
appending '!' to the option specification string, which was included in
perl-5.8.1 and is not present in perl-5.8.0. The previous commit bumped
the minimum supported Perl version to 5.8.1 so we no longer need to
provide the '--no-' variants for negatable options manually.
Teach `--git-completion-helper` to output the '--no-' options. They are
not included in the options hash and would otherwise be lost.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
git-send-email.perl | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index d75a4a33dd..f214bd4521 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -119,13 +119,16 @@ sub completion_helper {
foreach my $key (keys %$original_opts) {
unless (exists $not_for_completion{$key}) {
- $key =~ s/!$//;
+ my $negatable = ($key =~ s/!$//);
if ($key =~ /[:=][si]$/) {
$key =~ s/[:=][si]$//;
push (@send_email_opts, "--$_=") foreach (split (/\|/, $key));
} else {
push (@send_email_opts, "--$_") foreach (split (/\|/, $key));
+ if ($negatable) {
+ push (@send_email_opts, "--no-$_") foreach (split (/\|/, $key));
+ }
}
}
}
@@ -491,7 +494,6 @@ sub config_regexp {
"bcc=s" => \@getopt_bcc,
"no-bcc" => \$no_bcc,
"chain-reply-to!" => \$chain_reply_to,
- "no-chain-reply-to" => sub {$chain_reply_to = 0},
"sendmail-cmd=s" => \$sendmail_cmd,
"smtp-server=s" => \$smtp_server,
"smtp-server-option=s" => \@smtp_server_options,
@@ -506,36 +508,27 @@ sub config_regexp {
"smtp-auth=s" => \$smtp_auth,
"no-smtp-auth" => sub {$smtp_auth = 'none'},
"annotate!" => \$annotate,
- "no-annotate" => sub {$annotate = 0},
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
"header-cmd=s" => \$header_cmd,
"no-header-cmd" => \$no_header_cmd,
"suppress-from!" => \$suppress_from,
- "no-suppress-from" => sub {$suppress_from = 0},
"suppress-cc=s" => \@suppress_cc,
"signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
- "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
- "cc-cover|cc-cover!" => \$cover_cc,
- "no-cc-cover" => sub {$cover_cc = 0},
- "to-cover|to-cover!" => \$cover_to,
- "no-to-cover" => sub {$cover_to = 0},
+ "cc-cover!" => \$cover_cc,
+ "to-cover!" => \$cover_to,
"confirm=s" => \$confirm,
"dry-run" => \$dry_run,
"envelope-sender=s" => \$envelope_sender,
"thread!" => \$thread,
- "no-thread" => sub {$thread = 0},
"validate!" => \$validate,
- "no-validate" => sub {$validate = 0},
"transfer-encoding=s" => \$target_xfer_encoding,
"format-patch!" => \$format_patch,
- "no-format-patch" => sub {$format_patch = 0},
"8bit-encoding=s" => \$auto_8bit_encoding,
"compose-encoding=s" => \$compose_encoding,
"force" => \$force,
"xmailer!" => \$use_xmailer,
- "no-xmailer" => sub {$use_xmailer = 0},
"batch-size=i" => \$batch_size,
"relogin-delay=i" => \$relogin_delay,
"git-completion-helper" => \$git_completion_helper,
--
2.43.0.rc2
^ permalink raw reply related
* [PATCH v3 1/2] perl: bump the required Perl version to 5.8.1 from 5.8.0
From: Todd Zullinger @ 2023-11-16 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <20231116193014.470420-1-tmz@pobox.com>
The following commit will make use of a Getopt::Long feature which is
only present in Perl >= 5.8.1. Document that as the minimum version we
support.
Many of our Perl scripts will continue to run with 5.8.0 but this change
allows us to adjust them as needed without breaking any promises to our
users.
The Perl requirement was last changed in d48b284183 (perl: bump the
required Perl version to 5.8 from 5.6.[21], 2010-09-24). At that time,
5.8.0 was 8 years old. It is now over 21 years old.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
I debated changing all the 'use 5.008;' lines here, as most don't
actually require a newer Perl, but the previous bump did the same.
I can see the merit in either direction.
Changing it allows future contributors to be confident in relying on
5.8.1 features.
Not changing it allows anyone stuck on 5.8.0 to continue using the perl
scripts which don't actually require 5.8.1.
Tangentially, the Perl docs for 'use' function recommend against the
5.008001 form[1]:
Specifying VERSION as a numeric argument of the form 5.024001 should
generally be avoided as older less readable syntax compared to
v5.24.1. Before perl 5.8.0 released in 2002 the more verbose numeric
form was the only supported syntax, which is why you might see it in
older code.
use v5.24.1; # compile time version check
use 5.24.1; # ditto
use 5.024_001; # ditto; older syntax compatible with perl 5.6
I'm not enough of a Perl coder to have a strong preference or desire to
push for such a change, but I thought it was worth mentioning in case
others wonder why we're using the 5.008001 form.
[1] https://perldoc.perl.org/functions/use#use-VERSION
Documentation/CodingGuidelines | 2 +-
INSTALL | 2 +-
contrib/diff-highlight/DiffHighlight.pm | 2 +-
contrib/mw-to-git/Git/Mediawiki.pm | 2 +-
git-archimport.perl | 2 +-
git-cvsexportcommit.perl | 2 +-
git-cvsimport.perl | 2 +-
git-cvsserver.perl | 2 +-
git-send-email.perl | 4 ++--
git-svn.perl | 2 +-
gitweb/INSTALL | 2 +-
gitweb/gitweb.perl | 2 +-
perl/Git.pm | 2 +-
perl/Git/I18N.pm | 2 +-
perl/Git/LoadCPAN.pm | 2 +-
perl/Git/LoadCPAN/Error.pm | 2 +-
perl/Git/LoadCPAN/Mail/Address.pm | 2 +-
perl/Git/Packet.pm | 2 +-
t/t0202/test.pl | 2 +-
t/t5562/invoke-with-content-length.pl | 2 +-
t/t9700/test.pl | 2 +-
t/test-terminal.perl | 2 +-
22 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 8d3a467c01..39b9b7260f 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -490,7 +490,7 @@ For Perl programs:
- Most of the C guidelines above apply.
- - We try to support Perl 5.8 and later ("use Perl 5.008").
+ - We try to support Perl 5.8.1 and later ("use Perl 5.008001").
- use strict and use warnings are strongly preferred.
diff --git a/INSTALL b/INSTALL
index 4b42288882..06f29a8ae7 100644
--- a/INSTALL
+++ b/INSTALL
@@ -119,7 +119,7 @@ Issues of note:
- A POSIX-compliant shell is required to run some scripts needed
for everyday use (e.g. "bisect", "request-pull").
- - "Perl" version 5.8 or later is needed to use some of the
+ - "Perl" version 5.8.1 or later is needed to use some of the
features (e.g. sending patches using "git send-email",
interacting with svn repositories with "git svn"). If you can
live without these, use NO_PERL. Note that recent releases of
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index 376f577737..636add6968 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -1,6 +1,6 @@
package DiffHighlight;
-use 5.008;
+use 5.008001;
use warnings FATAL => 'all';
use strict;
diff --git a/contrib/mw-to-git/Git/Mediawiki.pm b/contrib/mw-to-git/Git/Mediawiki.pm
index 917d9e2d32..ff7811225e 100644
--- a/contrib/mw-to-git/Git/Mediawiki.pm
+++ b/contrib/mw-to-git/Git/Mediawiki.pm
@@ -1,6 +1,6 @@
package Git::Mediawiki;
-use 5.008;
+use 5.008001;
use strict;
use POSIX;
use Git;
diff --git a/git-archimport.perl b/git-archimport.perl
index b7c173c345..f5a317b899 100755
--- a/git-archimport.perl
+++ b/git-archimport.perl
@@ -54,7 +54,7 @@ =head1 Devel Notes
=cut
-use 5.008;
+use 5.008001;
use strict;
use warnings;
use Getopt::Std;
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 289d4bc684..1e03ba94d1 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -1,6 +1,6 @@
#!/usr/bin/perl
-use 5.008;
+use 5.008001;
use strict;
use warnings;
use Getopt::Std;
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 7bf3c12d67..07ea3443f7 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -13,7 +13,7 @@
# The head revision is on branch "origin" by default.
# You can change that with the '-o' option.
-use 5.008;
+use 5.008001;
use strict;
use warnings;
use Getopt::Long;
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 7b757360e2..124f598bdc 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -15,7 +15,7 @@
####
####
-use 5.008;
+use 5.008001;
use strict;
use warnings;
use bytes;
diff --git a/git-send-email.perl b/git-send-email.perl
index cacdbd6bb2..d75a4a33dd 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -16,7 +16,7 @@
# and second line is the subject of the message.
#
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
use Getopt::Long;
@@ -228,7 +228,7 @@ sub system_or_msg {
my @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code);
if (defined $msg) {
# Quiet the 'redundant' warning category, except we
- # need to support down to Perl 5.8, so we can't do a
+ # need to support down to Perl 5.8.1, so we can't do a
# "no warnings 'redundant'", since that category was
# introduced in perl 5.22, and asking for it will die
# on older perls.
diff --git a/git-svn.perl b/git-svn.perl
index 4e8878f035..b0d0a50984 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1,7 +1,7 @@
#!/usr/bin/perl
# Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
# License: GPL v2 or later
-use 5.008;
+use 5.008001;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
use strict;
use vars qw/ $AUTHOR $VERSION
diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index a58e6b3c44..dadc6efa81 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -29,7 +29,7 @@ Requirements
------------
- Core git tools
- - Perl 5.8
+ - Perl 5.8.1
- Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.
- web server
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e66eb3d9ba..55e7c6567e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7,7 +7,7 @@
#
# This program is licensed under the GPLv2
-use 5.008;
+use 5.008001;
use strict;
use warnings;
# handle ACL in file access tests
diff --git a/perl/Git.pm b/perl/Git.pm
index 117765dc73..03bf570bf4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -7,7 +7,7 @@ =head1 NAME
package Git;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index 895e759c57..5454c3a6d2 100644
--- a/perl/Git/I18N.pm
+++ b/perl/Git/I18N.pm
@@ -1,5 +1,5 @@
package Git::I18N;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
BEGIN {
diff --git a/perl/Git/LoadCPAN.pm b/perl/Git/LoadCPAN.pm
index 0c360bc799..8c7fa805f9 100644
--- a/perl/Git/LoadCPAN.pm
+++ b/perl/Git/LoadCPAN.pm
@@ -1,5 +1,5 @@
package Git::LoadCPAN;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
diff --git a/perl/Git/LoadCPAN/Error.pm b/perl/Git/LoadCPAN/Error.pm
index 5d84c20288..5cecb0fcd6 100644
--- a/perl/Git/LoadCPAN/Error.pm
+++ b/perl/Git/LoadCPAN/Error.pm
@@ -1,5 +1,5 @@
package Git::LoadCPAN::Error;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
use Git::LoadCPAN (
diff --git a/perl/Git/LoadCPAN/Mail/Address.pm b/perl/Git/LoadCPAN/Mail/Address.pm
index 340e88a7a5..9f808090a6 100644
--- a/perl/Git/LoadCPAN/Mail/Address.pm
+++ b/perl/Git/LoadCPAN/Mail/Address.pm
@@ -1,5 +1,5 @@
package Git::LoadCPAN::Mail::Address;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
use Git::LoadCPAN (
diff --git a/perl/Git/Packet.pm b/perl/Git/Packet.pm
index d144f5168f..d896e69523 100644
--- a/perl/Git/Packet.pm
+++ b/perl/Git/Packet.pm
@@ -1,5 +1,5 @@
package Git::Packet;
-use 5.008;
+use 5.008001;
use strict;
use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
BEGIN {
diff --git a/t/t0202/test.pl b/t/t0202/test.pl
index 2cbf7b9590..47d96a2a13 100755
--- a/t/t0202/test.pl
+++ b/t/t0202/test.pl
@@ -1,5 +1,5 @@
#!/usr/bin/perl
-use 5.008;
+use 5.008001;
use lib (split(/:/, $ENV{GITPERLLIB}));
use strict;
use warnings;
diff --git a/t/t5562/invoke-with-content-length.pl b/t/t5562/invoke-with-content-length.pl
index 718dd9b49d..9babb9a375 100644
--- a/t/t5562/invoke-with-content-length.pl
+++ b/t/t5562/invoke-with-content-length.pl
@@ -1,4 +1,4 @@
-use 5.008;
+use 5.008001;
use strict;
use warnings;
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 6d753708d2..d8e85482ab 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -1,7 +1,7 @@
#!/usr/bin/perl
use lib (split(/:/, $ENV{GITPERLLIB}));
-use 5.008;
+use 5.008001;
use warnings;
use strict;
diff --git a/t/test-terminal.perl b/t/test-terminal.perl
index 1bcf01a9a4..3810e9bb43 100755
--- a/t/test-terminal.perl
+++ b/t/test-terminal.perl
@@ -1,5 +1,5 @@
#!/usr/bin/perl
-use 5.008;
+use 5.008001;
use strict;
use warnings;
use IO::Pty;
--
2.43.0.rc2
^ permalink raw reply related
* [PATCH v3 0/2] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-16 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Ævar Arnfjörð Bjarmason,
Ondřej Pohořelský
In-Reply-To: <xmqq4jhmthtg.fsf@gitster.g>
Junio C Hamano wrote:
> Todd Zullinger <tmz@pobox.com> writes:
[...]
>> A little history:
>>
>> Support for the '--no-' prefix was added in Getopt::Long >= 2.33, in
>> commit 8ca8b48 (Negatable options (with "!") now also support the
>> "no-" prefix., 2003-04-04). Getopt::Long 2.34 was included in
>> perl-5.8.1 (2003-09-25), per Module::CoreList[1].
>>
>> We list perl-5.8 as the minimum version in INSTALL. This would leave
>> users with perl-5.8.0 (2002-07-18) with non-working arguments for
>> options where we're removing the explicit 'no-' variant.
>>
>> The explicit 'no-' opts were added in f471494303 (git-send-email.perl:
>> support no- prefix with older GetOptions, 2015-01-30), specifically to
>> support perl-5.8.0 which includes the older Getopt::Long.
>
> These are all very much relevant and deserve to be in the log
> message, not hidden under the three-dash line, I would think.
> Thanks for digging the history. The first paragraph was a bit hard
> to read as it wasn't clear "support" on which side is being
> discussed, though. If it were written perhaps like so:
>
> Getopt::Long >= 2.33 started supporting the '--no-' prefix
> natively by appending '!' to the option specification string,
> which was shipped with perl-5.8.1 and not present in perl-5.8.0
>
> it would have been clear that it was talking about the support
> given by Getopt module, not on our side.
That is much better. I've adjusted the commit message similarly and
hopefully kept your improved wording largely intact.
>> It may be time to bump the Perl requirement to 5.8.1 (2003-09-25) or
>> even 5.10.0 (2007-12-18). We last bumped the requirement from 5.6 to
>> 5.8 in d48b284183 (perl: bump the required Perl version to 5.8 from
>> 5.6.[21], 2010-09-24).
>
> Isn't the position this patch takes a lot stronger than "It may be
> time"? If we applied this patch, it drops the support for folks
> with Perl 5.8.0 (which I do not think is a bad thing, by the way).
Indeed it is. I should have mentioned that more explicitly. I added
the RFC tag to this round because I was unsure whether we'd want to go
the route of bumping the Perl requirement. But I managed to not
actually say as much.
> This sounds like something that is worth describing in the log
> message (and Release Notes).
I think the new commit messages describe the changes better. I didn't
include anything in RelNotes as I was presuming we'd leave this for
2.44 rather than risk causing any problems this late in the 2.43 cycle.
If you think the risk is low and/or the benefit is high, I can add it to
the 2.43.0 RelNotes.
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index cacdbd6bb2..94046e0fb7 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -119,13 +119,16 @@ sub completion_helper {
>>
>> foreach my $key (keys %$original_opts) {
>> unless (exists $not_for_completion{$key}) {
>> - $key =~ s/!$//;
>> + my $negate = ($key =~ s/!$//);
>
> A very minor nit, but I'd call this $negatable if I were doing this
> patch.
Sounds good.
> Just to make sure I did not misunderstand what you said below the
> three-dash line, if we were to take the other option that allows us
> to live with 5.8.0, we would make this hunk ...
>
>> "chain-reply-to!" => \$chain_reply_to,
>> - "no-chain-reply-to" => sub {$chain_reply_to = 0},
>
> ... look more like this?
>
>> - "chain-reply-to!" => \$chain_reply_to,
>> + "chain-reply-to" => \$chain_reply_to,
>> "no-chain-reply-to" => sub {$chain_reply_to = 0},
>> + "nochain-reply-to" => sub {$chain_reply_to = 0},
>
> That is, by removing the "!" suffix, we reject the native support of
> "--no-*" offered by Getopt::Long, and implement the negated variants
> ourselves?
Exactly. We could bundle the two no* options together, but that's a
trivial style issue, i.e.:
> - "chain-reply-to!" => \$chain_reply_to,
> - "no-chain-reply-to" => sub {$chain_reply_to = 0},
> + "chain-reply-to" => \$chain_reply_to,
> + "no-chain-reply-to|nochain-reply-to" => sub {$chain_reply_to = 0},
Thanks for a very helpful review, as always.
Todd Zullinger (2):
perl: bump the required Perl version to 5.8.1 from 5.8.0
send-email: avoid duplicate specification warnings
Documentation/CodingGuidelines | 2 +-
INSTALL | 2 +-
contrib/diff-highlight/DiffHighlight.pm | 2 +-
contrib/mw-to-git/Git/Mediawiki.pm | 2 +-
git-archimport.perl | 2 +-
git-cvsexportcommit.perl | 2 +-
git-cvsimport.perl | 2 +-
git-cvsserver.perl | 2 +-
git-send-email.perl | 23 ++++++++---------------
git-svn.perl | 2 +-
gitweb/INSTALL | 2 +-
gitweb/gitweb.perl | 2 +-
perl/Git.pm | 2 +-
perl/Git/I18N.pm | 2 +-
perl/Git/LoadCPAN.pm | 2 +-
perl/Git/LoadCPAN/Error.pm | 2 +-
perl/Git/LoadCPAN/Mail/Address.pm | 2 +-
perl/Git/Packet.pm | 2 +-
t/t0202/test.pl | 2 +-
t/t5562/invoke-with-content-length.pl | 2 +-
t/t9700/test.pl | 2 +-
t/test-terminal.perl | 2 +-
22 files changed, 29 insertions(+), 36 deletions(-)
Range-diff against v2:
-: ---------- > 1: b276216a53 perl: bump the required Perl version to 5.8.1 from 5.8.0
1: 59e2c79085 ! 2: e076a2ede5 send-email: avoid duplicate specification warnings
@@ Metadata
## Commit message ##
send-email: avoid duplicate specification warnings
- With perl-Getopt-Long >= 2.55 a warning is issued for options which are
- specified more than once. In addition to causing users to see warnings,
- this results in test failures which compare the output. An example,
- from t9001-send-email.37:
+ A warning is issued for options which are specified more than once
+ beginning with perl-Getopt-Long >= 2.55. In addition to causing users
+ to see warnings, this results in test failures which compare the output.
+ An example, from t9001-send-email.37:
| +++ diff -u expect actual
| --- expect 2023-11-14 10:38:23.854346488 +0000
@@ Commit message
| error: last command exited with $?=1
| not ok 37 - reject long lines
- Remove the duplicate option specs.
+ Remove the duplicate option specs. These are primarily the explicit
+ '--no-' prefix opts which were added in f471494303 (git-send-email.perl:
+ support no- prefix with older GetOptions, 2015-01-30). This was done
+ specifically to support perl-5.8.0 which includes Getopt::Long 2.32[1].
+
+ Getopt::Long 2.33 added support for the '--no-' prefix natively by
+ appending '!' to the option specification string, which was included in
+ perl-5.8.1 and is not present in perl-5.8.0. The previous commit bumped
+ the minimum supported Perl version to 5.8.1 so we no longer need to
+ provide the '--no-' variants for negatable options manually.
Teach `--git-completion-helper` to output the '--no-' options. They are
not included in the options hash and would otherwise be lost.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
## git-send-email.perl ##
@@ git-send-email.perl: sub completion_helper {
foreach my $key (keys %$original_opts) {
unless (exists $not_for_completion{$key}) {
- $key =~ s/!$//;
-+ my $negate = ($key =~ s/!$//);
++ my $negatable = ($key =~ s/!$//);
if ($key =~ /[:=][si]$/) {
$key =~ s/[:=][si]$//;
push (@send_email_opts, "--$_=") foreach (split (/\|/, $key));
} else {
push (@send_email_opts, "--$_") foreach (split (/\|/, $key));
-+ if ($negate) {
++ if ($negatable) {
+ push (@send_email_opts, "--no-$_") foreach (split (/\|/, $key));
+ }
}
2: c1f37d4395 < -: ---------- send-email: remove stray characters from usage
--
2.43.0.rc2
^ permalink raw reply
* Re: [PATCH 1/1] attr: add builtin objectmode values support
From: Joanna Wang @ 2023-11-16 17:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sunshine, tboegi
In-Reply-To: <xmqqil62qfvr.fsf@gitster.g>
yes sorry, t/#t1700 was a mistake.
Thank you for the review and further fixes
On Thu, Nov 16, 2023 at 12:08 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Other than the removal of that file, I locally applied the following
> > fix-up while checking the difference relative to the previous
> > iteration.
>
> Cumulatively, aside from the removal of the t/#t* file, here is what
> I ended up with so far.
>
> Subject: [PATCH] SQUASH???
>
> ---
> Documentation/gitattributes.txt | 2 +-
> neue | 0
> t/t0003-attributes.sh | 5 +++--
> t/t6135-pathspec-with-attrs.sh | 10 ++++++----
> 4 files changed, 10 insertions(+), 7 deletions(-)
> create mode 100644 neue
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 784aa9d4de..201bdf5edb 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -108,7 +108,7 @@ user defined attributes under this namespace will be ignored and
> trigger a warning.
>
> `builtin_objectmode`
> -^^^^^^^^^^^^^^^^^^^^
> +~~~~~~~~~~~~~~~~~~~~
> This attribute is for filtering files by their file bit modes (40000,
> 120000, 160000, 100755, 100644). e.g. ':(attr:builtin_objectmode=160000)'.
> You may also check these values with `git check-attr builtin_objectmode -- <file>`.
> diff --git a/neue b/neue
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> index 86f8681570..774b52c298 100755
> --- a/t/t0003-attributes.sh
> +++ b/t/t0003-attributes.sh
> @@ -580,12 +580,13 @@ test_expect_success 'builtin object mode attributes work (dir and regular paths)
> '
>
> test_expect_success POSIXPERM 'builtin object mode attributes work (executable)' '
> - >exec && chmod +x exec &&
> + >exec &&
> + chmod +x exec &&
> attr_check_object_mode exec 100755
> '
>
> test_expect_success SYMLINKS 'builtin object mode attributes work (symlinks)' '
> - >to_sym ln -s to_sym sym &&
> + ln -s to_sym sym &&
> attr_check_object_mode sym 120000
> '
>
> diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
> index b08a32ea68..f6403ebbda 100755
> --- a/t/t6135-pathspec-with-attrs.sh
> +++ b/t/t6135-pathspec-with-attrs.sh
> @@ -295,22 +295,24 @@ test_expect_success 'reading from .gitattributes in a subdirectory (3)' '
> test_cmp expect actual
> '
>
> -test_expect_success 'pathspec with builtin_objectmode attr can be used' '
> +test_expect_success POSIXPERM 'pathspec with builtin_objectmode attr can be used' '
> >mode_exec_file_1 &&
>
> git status -s ":(attr:builtin_objectmode=100644)mode_exec_*" >actual &&
> echo ?? mode_exec_file_1 >expect &&
> test_cmp expect actual &&
>
> - git add mode_exec_file_1 && chmod +x mode_exec_file_1 &&
> + git add mode_exec_file_1 &&
> + chmod +x mode_exec_file_1 &&
> git status -s ":(attr:builtin_objectmode=100755)mode_exec_*" >actual &&
> echo AM mode_exec_file_1 >expect &&
> test_cmp expect actual
> '
>
> -test_expect_success 'builtin_objectmode attr can be excluded' '
> +test_expect_success POSIXPERM 'builtin_objectmode attr can be excluded' '
> >mode_1_regular &&
> - >mode_1_exec && chmod +x mode_1_exec &&
> + >mode_1_exec &&
> + chmod +x mode_1_exec &&
> git status -s ":(exclude,attr:builtin_objectmode=100644)" "mode_1_*" >actual &&
> echo ?? mode_1_exec >expect &&
> test_cmp expect actual &&
> --
> 2.43.0-rc2
>
^ permalink raw reply
* Re: Feature request: git status --branch-only
From: phillip.wood123 @ 2023-11-16 16:09 UTC (permalink / raw)
To: Jeff King, phillip.wood; +Cc: Ondra Medek, git
In-Reply-To: <20231114201808.GE2092538@coredump.intra.peff.net>
Hi Peff
On 14/11/2023 20:18, Jeff King wrote:
> On Tue, Nov 14, 2023 at 03:02:04PM +0000, Phillip Wood wrote:
>
>> Hi Ondra
>>
>> On 14/11/2023 12:40, Ondra Medek wrote:
>>> Hi Phillip,
>>>
>>> it does not work for a fresh clone of an empty repository
>>>
>>> git for-each-ref --format="%(upstream:short)" refs/heads/master
>>>
>>> outputs nothing, while
>>
>> Oh dear, that's a shame. I wonder if it is a bug because the documentation
>> says that
>>
>> --format="%(upstream:track)"
>>
>> should print "[gone]" whenever an unknown upstream ref is encountered but
>> trying that on a clone of an empty repository gives no output.
>
> I think it would print "gone" if the upstream branch went missing. But
> in this case the actual local branch is missing. And for-each-ref will
> not show an entry at all for a ref that does not exist. The
> "refs/heads/master" on your command line is not a ref, but a pattern,
> and that pattern does not match anything. So it's working as intended.
Oh of course, I'd somehow forgotten that "refs/heads/master" did not
exist so it makes sense that for-each-ref does not print anything.
> I think a more direct tool would be:
>
> git rev-parse --symbolic-full-name master@{upstream}
>
> That convinces branch_get_upstream() to return the value we want, but
> sadly it seems to get lost somewhere in the resolution process, and we
> spit out an error. Arguably that is a bug (with --symbolic or
> --symbolic-full-name, I think it would be OK to resolve names even if
> they don't point to something, but it's possible that would have other
> unexpected side effects).
Yeah, maybe we should look at fixing that - I didn't suggest it because
I knew it did not work on an unborn branch but as you say there is no
obvious reason why it shouldn't
Best Wishes
Phillip
> -Peff
^ permalink raw reply
* Re: Feasibility of folding `unit-tests` into `make test`, was Re: [PATCH] ci: avoid running the test suite _twice_
From: phillip.wood123 @ 2023-11-16 15:05 UTC (permalink / raw)
To: Johannes Schindelin, Josh Steadmon
Cc: Jeff King, Johannes Schindelin via GitGitGadget, Phillip Wood,
git
In-Reply-To: <850ea42c-f103-68d5-896b-9120e2628686@gmx.de>
On 16/11/2023 08:42, Johannes Schindelin wrote:
> On Wed, 15 Nov 2023, Josh Steadmon wrote:
>> On 2023.11.13 13:49, Jeff King wrote:
>> We could bundle all the unit tests into a single shell script, but then
>> we lose parallelization and add hoops to jump through to determine what
>> breaks. Or we could autogenerate a corresponding shell script to run
>> each individual unit test, but that seems gross. Of course, these are
>> hypothetical concerns for now, since we only have a single unit test at
>> the moment.
>
> I totally agree with you, Josh, that it makes little sense to
> try to contort the unit tests to be run in the same `prove` run as the
> regression tests that need to be invoked so totally differently.
FWIW that's my feeling too. It makes sense for "make test" to run the
unit tests, but wrapping the unit tests in one or more shell scripts
adds unnecessary complexity.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 02/10] ref-filter.h: add max_count and omit_empty to ref_format
From: Øystein Walle @ 2023-11-16 12:06 UTC (permalink / raw)
To: gitgitgadget; +Cc: code, git, oystwa, ps, vdye
In-Reply-To: <adac101bc6022d5477371d6a94225f38da7fffee.1699991638.git.gitgitgadget@gmail.com>
Victoria Dye <vdye@github.com> writes:
> diff --git a/ref-filter.h b/ref-filter.h
> index 1524bc463a5..d87d61238b7 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -92,6 +92,11 @@ struct ref_format {
>
> /* List of bases for ahead-behind counts. */
> struct string_list bases;
> +
> + struct {
> + int max_count;
> + int omit_empty;
> + } array_opts;
> };
What the benefit of having them in a nested struct is compared to just
two distinct members?
Regardless this is the kind of deduplication I wanted to achieve when I
added --omit-empty, but never did. Either way, I meant to ack this in
the last round never got around to it. Nice work.
Acked-by: Øystein Walle <oystwa@gmail.com>
Øsse
^ permalink raw reply
* Re: [PATCH] commit-graph: disable GIT_COMMIT_GRAPH_PARANOIA by default
From: Patrick Steinhardt @ 2023-11-16 11:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, Karthik Nayak
In-Reply-To: <xmqqh6lmwogq.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]
On Thu, Nov 16, 2023 at 09:07:01AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> >> Yeah. Just like we auto-enabled GIT_REF_PARANOIA for git-gc, etc, I
> >> think we should do the same here.
> >
> > I'm honestly still torn on this one. There are two cases that I can
> > think of where missing objects would be benign and where one wants to
> > use `git rev-list --missing`:
> >
> > - Repositories with promisor remotes, to find the boundary of where
> > we need to fetch new objects.
> >
> > - Quarantine directories where you only intend to list new objects
> > or find the boundary.
> >
> > And in neither of those cases I can see a path for how the commit-graph
> > would contain such missing commits when using regular tooling to perform
> > repository maintenance.
>
> I can buy the promisor remotes use case---we may expect boundary
> objects missing without any repository corruption. I do not know
> about the other one---where does our "rev-list --missing" start
> traversing in a quarantine directory is unclear. Objects that are
> still in quarantine are not (yet) made reachable from any refs, so
> even "rev-list --missing --all" would not make a useful traversal,
> no?
You typically know about the new tips when having a quarantine
directory. So you can discover the boundary between objects in your
quarantine directory and your main object database by executing `git
rev-list $NEWREVS --missing=print` and execute that command with
`GIT_OBJECT_DIRECTORY=$quarantine`. The benefit is that this scales with
the number of objects in the quarantine, and not with the size of the
overall repository.
As mention, this is really niche, but we do plan to use this in Gitaly
eventually.
> In any case, it sounds like you are not torn but are convinced that
> we should leave this loose by default ;-) and after thinking it over
> again, I tend to agree that it would be a better choice, as long as
> the feature "rev-list --missing" has any good use case other than
> corruption in repository.
>
> So,... thanks for pushing back.
Okay, glad to hear that I'm not totally bonkers then. I'm going to wait
another few days for additional feedback before sending a v2. But if
what I'm saying also makes sense to others then v2 would only squash in
the diff I sent to run the subset of tests that is now failing with
`GIT_COMMIT_GRAPH_PARANOIA=true`.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v7 00/14] Introduce new `git replay` command
From: Johannes Schindelin @ 2023-11-16 8:53 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>
Hi Christian,
[focusing exclusively on the `range-diff` because I lack the capacity for
anything beyond that]
On Wed, 15 Nov 2023, Christian Couder wrote:
> # Range-diff between v6 and v7
>
> (Sorry it looks like patch 6/14 in v7 is considered to be completely
> different from what it was in v6, so the range-diff is not showing
> differences between them.)
>
> 1: fac0a9dff4 = 1: cddcd967b2 t6429: remove switching aspects of fast-rebase
> 2: bec2eb8928 ! 2: c8476fb093 replay: introduce new builtin
> @@ Documentation/git-replay.txt (new)
> +DESCRIPTION
> +-----------
> +
> -+Takes a range of commits and replays them onto a new location.
> ++Takes a range of commits, specified by <oldbase> and <branch>, and
> ++replays them onto a new location (see `--onto` option below).
> +
> +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> +
Thank you.
> 3: b0cdfdc0c3 = 3: 43322abd1e replay: start using parse_options API
> 4: c3403f0b9d = 4: 6524c7f045 replay: die() instead of failing assert()
> 5: 4188eeac30 = 5: 05d0efa3cb replay: introduce pick_regular_commit()
> 6: b7b4d9001e < -: ---------- replay: change rev walking options
> -: ---------- > 6: c7a5aad3d6 replay: change rev walking options
The actual range-diff for the sixth patch looks like this:
-- snip --
6: b7b4d9001e9 ! 6: c7a5aad3d62 replay: change rev walking options
@@ Metadata
## Commit message ##
replay: change rev walking options
- Let's set the rev walking options we need after calling
- setup_revisions() instead of before. This enforces options we always
- want for now.
+ Let's force the rev walking options we need after calling
+ setup_revisions() instead of before.
+
+ This might override some user supplied rev walking command line options
+ though. So let's detect that and warn users by:
+
+ a) setting the desired values, before setup_revisions(),
+ b) checking after setup_revisions() whether these values differ from
+ the desired values,
+ c) if so throwing a warning and setting the desired values again.
We want the command to work from older commits to newer ones by default.
Also we don't want history simplification, as we want to deal with all
the commits in the affected range.
- When we see an option that we are going to override, we emit a warning
- to avoid confusion as much as possible though.
-
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
@@ Commit message
## builtin/replay.c ##
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
- struct merge_result result;
- struct strbuf reflog_msg = STRBUF_INIT;
- struct strbuf branch_name = STRBUF_INIT;
-- int ret = 0;
-+ int i, ret = 0;
-
- const char * const replay_usage[] = {
- N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
-@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
repo_init_revisions(the_repository, &revs, prefix);
@@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
- revs.max_parents = 1;
- revs.cherry_mark = 1;
- revs.limited = 1;
-- revs.reverse = 1;
++ strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
++
++ /*
++ * Set desired values for rev walking options here. If they
++ * are changed by some user specified option in setup_revisions()
++ * below, we will detect that below and then warn.
++ *
++ * TODO: In the future we might want to either die(), or allow
++ * some options changing these values if we think they could
++ * be useful.
++ */
+ revs.reverse = 1;
- revs.right_only = 1;
-- revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
-- revs.topo_order = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.topo_order = 1;
-
- strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
+- strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
++ revs.simplify_history = 0;
-+ /*
-+ * TODO: For now, let's warn when we see an option that we are
-+ * going to override after setup_revisions() below. In the
-+ * future we might want to either die() or allow them if we
-+ * think they could be useful though.
-+ */
-+ for (i = 0; i < argc; i++) {
-+ if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
-+ !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
-+ !strcmp(argv[i], "--full-history"))
-+ warning(_("option '%s' will be overridden"), argv[i]);
-+ }
-+
if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
ret = error(_("unhandled options"));
goto cleanup;
}
-+ /* requirements/overrides for revs */
-+ revs.reverse = 1;
-+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
-+ revs.topo_order = 1;
-+ revs.simplify_history = 0;
++ /*
++ * Detect and warn if we override some user specified rev
++ * walking options.
++ */
++ if (revs.reverse != 1) {
++ warning(_("some rev walking options will be overridden as "
++ "'%s' bit in 'struct rev_info' will be forced"),
++ "reverse");
++ revs.reverse = 1;
++ }
++ if (revs.sort_order != REV_SORT_IN_GRAPH_ORDER) {
++ warning(_("some rev walking options will be overridden as "
++ "'%s' bit in 'struct rev_info' will be forced"),
++ "sort_order");
++ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
++ }
++ if (revs.topo_order != 1) {
++ warning(_("some rev walking options will be overridden as "
++ "'%s' bit in 'struct rev_info' will be forced"),
++ "topo_order");
++ revs.topo_order = 1;
++ }
++ if (revs.simplify_history != 0) {
++ warning(_("some rev walking options will be overridden as "
++ "'%s' bit in 'struct rev_info' will be forced"),
++ "simplify_history");
++ revs.simplify_history = 0;
++ }
+
strvec_clear(&rev_walk_args);
-- snap --
This looks really good. Thank you for going the extra step to make this
patch so much better.
> 7: c57577a9b8 = 7: 01f35f924b replay: add an important FIXME comment about gpg signing
> 8: e78be50f3d = 8: 1498b24bad replay: remove progress and info output
> 9: e4c79b676f = 9: 6786fc147b replay: remove HEAD related sanity check
> 10: 8d89f1b733 ! 10: 9a24dbb530 replay: make it a minimal server side command
> @@ Commit message
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> ## Documentation/git-replay.txt ##
> -@@ Documentation/git-replay.txt: SYNOPSIS
> - DESCRIPTION
> +@@ Documentation/git-replay.txt: DESCRIPTION
> -----------
>
> --Takes a range of commits and replays them onto a new location.
> -+Takes a range of commits and replays them onto a new location. Leaves
> -+the working tree and the index untouched, and updates no
> -+references. The output of this command is meant to be used as input to
> + Takes a range of commits, specified by <oldbase> and <branch>, and
> +-replays them onto a new location (see `--onto` option below).
> ++replays them onto a new location (see `--onto` option below). Leaves
> ++the working tree and the index untouched, and updates no references.
> ++The output of this command is meant to be used as input to
> +`git update-ref --stdin`, which would update the relevant branches.
>
> THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> struct merge_result result;
> - struct strbuf reflog_msg = STRBUF_INIT;
> struct strbuf branch_name = STRBUF_INIT;
> - int i, ret = 0;
> + int ret = 0;
>
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
> onto = peel_committish(onto_name);
Looks good to me.
> 11: 3d433a1322 ! 11: ad6ca2fbef replay: use standard revision ranges
> @@ Documentation/git-replay.txt: git-replay - EXPERIMENTAL: Replay commits on a new
>
> DESCRIPTION
> -----------
> -@@ Documentation/git-replay.txt: DESCRIPTION
> - Takes a range of commits and replays them onto a new location. Leaves
> - the working tree and the index untouched, and updates no
> - references. The output of this command is meant to be used as input to
> +
> +-Takes a range of commits, specified by <oldbase> and <branch>, and
> +-replays them onto a new location (see `--onto` option below). Leaves
> ++Takes ranges of commits and replays them onto a new location. Leaves
> + the working tree and the index untouched, and updates no references.
> + The output of this command is meant to be used as input to
> -`git update-ref --stdin`, which would update the relevant branches.
> +`git update-ref --stdin`, which would update the relevant branches
> +(see the OUTPUT section below).
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> struct merge_options merge_opt;
> struct merge_result result;
> - struct strbuf branch_name = STRBUF_INIT;
> - int i, ret = 0;
> + int ret = 0;
>
> const char * const replay_usage[] = {
> - N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> - strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
> -
> /*
> - * TODO: For now, let's warn when we see an option that we are
> - * going to override after setup_revisions() below. In the
> + * Set desired values for rev walking options here. If they
> + * are changed by some user specified option in setup_revisions()
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
> - warning(_("option '%s' will be overridden"), argv[i]);
> - }
> + revs.topo_order = 1;
> + revs.simplify_history = 0;
>
> - if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
> - ret = error(_("unhandled options"));
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> }
>
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
> - revs.topo_order = 1;
> - revs.simplify_history = 0;
> + revs.simplify_history = 0;
> + }
>
> - strvec_clear(&rev_walk_args);
> -
This is the correct spot for that documentation change. Good.
> 12: cca8105382 ! 12: 081864ed5f replay: add --advance or 'cherry-pick' mode
> @@ builtin/replay.c: static struct commit *pick_regular_commit(struct commit *pickm
> struct merge_options merge_opt;
> struct merge_result result;
> + struct strset *update_refs = NULL;
> - int i, ret = 0;
> + int ret = 0;
>
> const char * const replay_usage[] = {
> - N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
>
> /*
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
> - revs.topo_order = 1;
> - revs.simplify_history = 0;
> + revs.simplify_history = 0;
> + }
>
> + determine_replay_mode(&revs.cmdline, onto_name, &advance_name,
> + &onto, &update_refs);
> 13: 92287a2cc8 ! 13: 19c4016c7c replay: add --contained to rebase contained branches
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> struct rev_info revs;
> struct commit *last_commit = NULL;
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
> - int i, ret = 0;
> + int ret = 0;
>
> const char * const replay_usage[] = {
> - N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
> 14: 529a7fda40 ! 14: 29556bcc86 replay: stop assuming replayed branches do not diverge
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> struct merge_result result;
> struct strset *update_refs = NULL;
> + kh_oid_map_t *replayed_commits;
> - int i, ret = 0;
> + int ret = 0;
>
> const char * const replay_usage[] = {
> @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix)
The last three only have context-only changes. Obviously good.
Apart from the one little outstanding nit where I would love to see
`(EXPERIMENTAL!)` as the first word of the synopsis both in the manual
page and in the output of `git replay -h`, you have addressed all of my
concerns.
Thank you!
Johannes
^ permalink raw reply
* Re: [PATCH v6 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-16 8:52 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <0ddca907-6e64-b684-2e08-c7e95e737a3c@gmx.de>
Hi Dscho,
On Thu, Nov 16, 2023 at 9:45 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Wed, 15 Nov 2023, Christian Couder wrote:
> > I have fixed that in the v7 I just sent with the following:
> >
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
>
> I still think that the following would serve us better:
>
> [verse]
> (EXPERIMENTAL!) 'git replay' --onto <newbase> <oldbase> <branch>
>
> But if nobody else feels as strongly, I won't bring this up again.
For the tests to pass, the SYNOPSIS should be the same as the first
line of the `git replay -h` output. After this series it is:
usage: git replay ([--contained] --onto <newbase> | --advance
<branch>) <revision-range>... # EXPERIMENTAL
As the usage is supposed to describe how the command should be used, I
think it makes sense for "EXPERIMENTAL" to be in a shell comment after
the command.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH v6 00/14] Introduce new `git replay` command
From: Johannes Schindelin @ 2023-11-16 8:45 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <CAP8UFD0Es4qai98WB6bpykisBT628JndPXG8jg1=_uUbn4zogA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]
Hi Christian,
On Wed, 15 Nov 2023, Christian Couder wrote:
> On Wed, Nov 8, 2023 at 1:47 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Thu, 2 Nov 2023, Christian Couder wrote:
>
> > > + ## Documentation/git-replay.txt (new) ##
> > > +@@
> > > ++git-replay(1)
> > > ++=============
> > > ++
> > > ++NAME
> > > ++----
> > > ++git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos too
> > > ++
> > > ++
> > > ++SYNOPSIS
> > > ++--------
> > > ++[verse]
> > > ++'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
> >
> > Technically, at this stage `git replay` requires precisely 5 arguments, so
> > the `<revision>...` is incorrect and should be `<upstream> <branch>`
> > instead. But it's not worth a new iteration to fix this.
>
> It was actually:
>
> 'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
Right.
> > > ++
> > > ++DESCRIPTION
> > > ++-----------
> > > ++
> > > ++Takes a range of commits and replays them onto a new location.
> > > ++
> > > ++THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> > > ++
> > > ++OPTIONS
> > > ++-------
> > > ++
> > > ++--onto <newbase>::
> > > ++ Starting point at which to create the new commits. May be any
> > > ++ valid commit, and not just an existing branch name.
> > > ++
> >
> > Traditionally, this would be a place to describe the `<revision>` argument
> > (or, in this patch, to reflect the current state of `builtin/replay.c`,
> > the `<upstream> <branch>` arguments instead).
>
> I have fixed that in the v7 I just sent with the following:
>
> +SYNOPSIS
> +--------
> +[verse]
> +'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
I still think that the following would serve us better:
[verse]
(EXPERIMENTAL!) 'git replay' --onto <newbase> <oldbase> <branch>
But if nobody else feels as strongly, I won't bring this up again.
Ciao,
Johannes
^ permalink raw reply
* Feasibility of folding `unit-tests` into `make test`, was Re: [PATCH] ci: avoid running the test suite _twice_
From: Johannes Schindelin @ 2023-11-16 8:42 UTC (permalink / raw)
To: Josh Steadmon
Cc: Jeff King, Johannes Schindelin via GitGitGadget, Phillip Wood,
git
In-Reply-To: <ZVU4EVcj0MDrSNcG@google.com>
[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]
Hi Josh,
On Wed, 15 Nov 2023, Josh Steadmon wrote:
> On 2023.11.13 13:49, Jeff King wrote:
> >
> > why are the unit tests totally separate from the rest of the suite? I
> > would think we'd want them run from one or more t/t*.sh scripts. That
> > would make bugs like this impossible, but also:
> >
> > 1. They'd be run via "make test", so developers don't have to remember
> > to run them separately.
> >
> > 2. They can be run in parallel with all of the other tests when using
> > "prove -j", etc.
>
> The first part is easy, but I don't see a good way to get both shell
> tests and unit tests executing under the same `prove` process. For shell
> tests, we pass `--exec '$(TEST_SHELL_PATH_SQ)'` to prove, meaning that
> we use the specified shell as an interpreter for the test files. That
> will not work for unit test executables.
Probably my favorite aspect about the new unit tests is that they avoid
using the error-prone, unintuitive and slow shell scripts and stay within
the programming language of the code that is to be tested: C.
> We could bundle all the unit tests into a single shell script, but then
> we lose parallelization and add hoops to jump through to determine what
> breaks. Or we could autogenerate a corresponding shell script to run
> each individual unit test, but that seems gross. Of course, these are
> hypothetical concerns for now, since we only have a single unit test at
> the moment.
I totally agree with you, Josh, that it makes little sense to
try to contort the unit tests to be run in the same `prove` run as the
regression tests that need to be invoked so totally differently.
> There's also the issue that the shell test arguments we pass on from
> prove would be shared with the unit tests. That's fine for now, as
> t-strbuf doesn't accept any runtime arguments, but it's possible that
> either the framework or individual unit tests might grow to need
> arguments, and it might not be convenient to stay compatible with the
> shell tests.
>
> Personally, I lean towards keeping things simple and just running a
> second `prove` process as part of `make test`.
Agreed.
> If I was forced to pick a way to get everything under one process, I'd
> lean towards autogenerating individual shell script wrappers for each
> unit test. But I'm open to discussion, especially if people have other
> approaches I haven't thought of.
One alternative would be to avoid running the unit tests via `prove` in
the first place.
For example, we could use the helper from be5d88e11280 (test-tool
run-command: learn to run (parts of) the testsuite, 2019-10-04) [*1*]. It
would probably need a few improvements, but certainly no wizardry nor
witchcraft would be required. It would also help on Windows, where running
a simple test helper written in C is vastly faster than running a complex
Perl script (which `prove` is).
Ciao,
Johannes
Footnote *1*: I had always wanted to improve that test helper to the point
where it could replace our use of `prove`, at least on Windows. It seems,
however, that as of 4c2c38e800f3 (ci: modification of main.yml to use
cmake for vs-build job, 2020-06-26) we do not use the helper at all
anymore. Hopefully it can still be useful. 🤞
^ permalink raw reply
* Re: [PATCH 1/1] attr: add builtin objectmode values support
From: Junio C Hamano @ 2023-11-16 8:08 UTC (permalink / raw)
To: Joanna Wang; +Cc: git, sunshine, tboegi
In-Reply-To: <xmqqsf56ql14.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Other than the removal of that file, I locally applied the following
> fix-up while checking the difference relative to the previous
> iteration.
Cumulatively, aside from the removal of the t/#t* file, here is what
I ended up with so far.
Subject: [PATCH] SQUASH???
---
Documentation/gitattributes.txt | 2 +-
neue | 0
t/t0003-attributes.sh | 5 +++--
t/t6135-pathspec-with-attrs.sh | 10 ++++++----
4 files changed, 10 insertions(+), 7 deletions(-)
create mode 100644 neue
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 784aa9d4de..201bdf5edb 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -108,7 +108,7 @@ user defined attributes under this namespace will be ignored and
trigger a warning.
`builtin_objectmode`
-^^^^^^^^^^^^^^^^^^^^
+~~~~~~~~~~~~~~~~~~~~
This attribute is for filtering files by their file bit modes (40000,
120000, 160000, 100755, 100644). e.g. ':(attr:builtin_objectmode=160000)'.
You may also check these values with `git check-attr builtin_objectmode -- <file>`.
diff --git a/neue b/neue
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 86f8681570..774b52c298 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -580,12 +580,13 @@ test_expect_success 'builtin object mode attributes work (dir and regular paths)
'
test_expect_success POSIXPERM 'builtin object mode attributes work (executable)' '
- >exec && chmod +x exec &&
+ >exec &&
+ chmod +x exec &&
attr_check_object_mode exec 100755
'
test_expect_success SYMLINKS 'builtin object mode attributes work (symlinks)' '
- >to_sym ln -s to_sym sym &&
+ ln -s to_sym sym &&
attr_check_object_mode sym 120000
'
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index b08a32ea68..f6403ebbda 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -295,22 +295,24 @@ test_expect_success 'reading from .gitattributes in a subdirectory (3)' '
test_cmp expect actual
'
-test_expect_success 'pathspec with builtin_objectmode attr can be used' '
+test_expect_success POSIXPERM 'pathspec with builtin_objectmode attr can be used' '
>mode_exec_file_1 &&
git status -s ":(attr:builtin_objectmode=100644)mode_exec_*" >actual &&
echo ?? mode_exec_file_1 >expect &&
test_cmp expect actual &&
- git add mode_exec_file_1 && chmod +x mode_exec_file_1 &&
+ git add mode_exec_file_1 &&
+ chmod +x mode_exec_file_1 &&
git status -s ":(attr:builtin_objectmode=100755)mode_exec_*" >actual &&
echo AM mode_exec_file_1 >expect &&
test_cmp expect actual
'
-test_expect_success 'builtin_objectmode attr can be excluded' '
+test_expect_success POSIXPERM 'builtin_objectmode attr can be excluded' '
>mode_1_regular &&
- >mode_1_exec && chmod +x mode_1_exec &&
+ >mode_1_exec &&
+ chmod +x mode_1_exec &&
git status -s ":(exclude,attr:builtin_objectmode=100644)" "mode_1_*" >actual &&
echo ?? mode_1_exec >expect &&
test_cmp expect actual &&
--
2.43.0-rc2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox