* Re: Git Push Always uses Protocol Version 0
From: Zsolt Imre @ 2024-01-22 19:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqa5oxurnj.fsf@gitster.g>
Hi. Thanks for your response. Yes, I saw those switch statements, but probably misunderstood the intent behind them.
Checking `determine_protocol_version_server` the in-code documentation says:
* Used by a server to determine which protocol version should be used based on
* a client's request, communicated via the 'GIT_PROTOCOL' environment variable
I explicitly set the `GIT_PROTOCOL` environment variable to `version=2`. (Will share more at the end of this email why, as you asked anyways)
The client-side calls `discover_version` to check the version supported by the server. Which ultimately ends up being done as:
enum protocol_version determine_protocol_version_client(const char *server_response)
{
enum protocol_version version = protocol_v0;
if (skip_prefix(server_response, "version ", &server_response)) {
version = parse_protocol_version(server_response);
if (version == protocol_unknown_version)
die("server is speaking an unknown protocol");
if (version == protocol_v0)
die("protocol error: server explicitly said version 0");
}
return version;
}
The server, according to my understanding of the documentations so far, will not return a version identifier if the client is not talking the version 2 (or maybe even version 1) of the protocol.
The git clone I mentioned in my previous email was clearly using protocol version 2 and it set the appropriate header to indicate this when discovering capabilities. See full request headers below.
POST /testing.git/git-upload-pack HTTP/1.1
Host: 127.0.0.1:9000
Accept: application/x-git-upload-pack-result
Accept-Encoding: deflate, gzip
Accept-Language: en-GB, *;q=0.9
Content-Length: 122
Content-Type: application/x-git-upload-pack-request
Git-Protocol: version=2
User-Agent: git/2.43.0
The server, in this case did not return a version identifier as the first PKT-LINE either, but responded otherwise according to the V2 protocol. Everything was working well.
Then, the above request was followed by by a proper fetch command in which the client again specified the use of V2 protocol:
POST /testing.git/git-upload-pack HTTP/1.1
Host: 127.0.0.1:9000
Accept: application/x-git-upload-pack-result
Accept-Encoding: deflate, gzip
Accept-Language: en-GB, *;q=0.9
Content-Length: 160
Content-Type: application/x-git-upload-pack-request
Git-Protocol: version=2
User-Agent: git/2.43.0
Accordingly, the cloning worked perfectly fine.
But then, when I want to push to the repo, the client does not do any capabilities or version discovery, just goes with V0 of the protocol to get the refs:
GET /testing.git/info/refs?service=git-receive-pack HTTP/1.1
Host: 127.0.0.1:9000
Accept: */*
Accept-Encoding: deflate, gzip
Accept-Language: en-GB, *;q=0.9
Cache-Control: no-cache
Pragma: no-cache
User-Agent: git/2.43.0
What I was expecting, given things are going stateless, that on push the client first discovers the protocol supported by the server and picks the most recent - in this case, that would be version 2.
And to answer your question of what I cannot do with the "current versions" of the protocol: I could do everything, of course. But, if there's protocol 0, 1 and 2 and I wanted to implement only version 2, I thought I should be able to. If protocol V2 was complete, I would not have to worry about implementing V0 and V1 (saving some time and headache), especially because I do not care about supporting old clients. I may have misunderstood the word "version" and version 2 is more of an "extension" to V1?
> On 22 Jan 2024, at 18:52, Junio C Hamano <gitster@pobox.com> wrote:
>
> Zsolt Imre <imrexzsolt@gmail.com> writes:
>
>> I'm not entirely sure if this is a bug or I am missing something,
>> but I thought I would share in the hope someone can help out. I'm
>> playing around with Git and trying to implement a git server that
>> communicates over HTTP and supports Git protocol version 2 *only*.
>>
>> When I `clone` a repository, the Git client (version 2.43.0),
>> after fetching the capabilities using protocol version 2, it
>> proceeds to fetch the refs, again, via protocol version 2 using
>> the `ls-refs` command. However, when I try to `push` my changes
>> to the repo, the Git client refuses to use protocol version 2 and
>> tries to obtain the ref list using protocol version 0, even if I
>> pass in the `-c protocol.version=2` command line argument.
>
> Given that v0 and v1 in the push direction behave exactly the same,
> and there has been no need to add features that were not supportable
> in v1 in the push direction, it is not surprising to see this code
>
> int cmd_send_pack(int argc, const char **argv, const char *prefix)
> {
> ...
> switch (discover_version(&reader)) {
> case protocol_v2:
> die("support for protocol v2 not implemented yet");
> break;
>
> in https://github.com/git/git/blob/master/builtin/send-pack.c#L282
> and also
>
> int cmd_receive_pack(int argc, const char **argv, const char *prefix)
> {
> ...
> switch (determine_protocol_version_server()) {
> case protocol_v2:
> /*
> * push support for protocol v2 has not been implemented yet,
> * so ignore the request to use v2 and fallback to using v0.
> */
> break;
>
> in https://github.com/git/git/blob/master/builtin/receive-pack.c#L2538
>
> that tells the receiving end to demote "v2" request down to "v0",
> and have the pushing end honor that choice.
>
> What specifically did you want to gain by using protocol version 2
> in the "push" direction that you cannot do with the current
> versions?
>
>
^ permalink raw reply
* Re: [PATCH 2/2] patch-id: replace `atoi()` with `strtol_i2()`
From: Junio C Hamano @ 2024-01-22 19:32 UTC (permalink / raw)
To: Mohit Marathe via GitGitGadget; +Cc: git, Mohit Marathe, Mohit Marathe
In-Reply-To: <1ece724b1ca7f71542bfef42795fce798563ecde.1705913519.git.gitgitgadget@gmail.com>
"Mohit Marathe via GitGitGadget" <gitgitgadget@gmail.com> writes:
> static const char digits[] = "0123456789";
> const char *q, *r;
> + char *endp;
> int n;
>
> q = p + 4;
> n = strspn(q, digits);
> if (q[n] == ',') {
> q += n + 1;
> - *p_before = atoi(q);
> + if (strtol_i2(q, 10, p_before, &endp) != 0)
> + return 0;
> n = strspn(q, digits);
> } else {
> *p_before = 1;
> }
Looking at this code again, because we upfront run strspn() to make
sure q[] begins with a run of digits *and* followed by a comma
(which is not a digit), I think it is safe to use atoi() and assume
it would slurp all the digits. So the lack of another check the use
of new helper allows us to do, namely
if (endp != q + n)
return 0;
is probably OK, but that is one of the two reasons why you would
favor the use of new helper over atoi(), so the upside of this
change is not all that great as I originally hoped for X-<.
Not your fault, of course. We would still catch when the digit
string that starts q[] is too large to fit in an int, which is an
upside.
> - if (n == 0 || q[n] != ' ' || q[n+1] != '+')
> + if (q[n] != ' ' || q[n+1] != '+')
> return 0;
When we saw q[] that begins with ',' upon entry to this function, we
used to say *p_before = 1 and then saw n==0 and realized it is not a
good input and returned 0 from the function.
Now we instead peek q[0] and the check says q[0] is not SP so we
will return 0 the same way so there is no behaviour change from the
upper hunk? The conversion may be correct, but it wasn't explained
in the proposed commit log message.
How are the change to stop caring about n==0 here ...
> r = q + n + 2;
> n = strspn(r, digits);
> if (r[n] == ',') {
> r += n + 1;
> - *p_after = atoi(r);
> - n = strspn(r, digits);
> + if (strtol_i2(r, 10, p_after, &endp) != 0)
> + return 0;
> } else {
> *p_after = 1;
> }
> - if (n == 0)
> - return 0;
... and this change here, linked to the switch from atoi() to
strtul_i2()[*]?
It looks like an unrelated behaviour change that is left
unexplained.
> return 1;
> }
Thanks for working on this one.
[Footnote]
* by the way, what a horrible name for a public function. Yuck.
^ permalink raw reply
* Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs
From: Phillip Wood @ 2024-01-22 19:54 UTC (permalink / raw)
To: Patrick Steinhardt, Junio C Hamano; +Cc: git
In-Reply-To: <Za5IoEjs0q7cf354@tanuki>
On 22/01/2024 10:51, Patrick Steinhardt wrote:
> On Fri, Jan 19, 2024 at 12:09:25PM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> The conversion (rather, the introduction to allow refs API to be
>> used to access them) look correct, but offhand I do not know what
>> the implication of leaving the file based interface would be.
>
> We have said in past discussions that the sequencer state should remain
> self contained, and that requires us to keep the files-based interface.
> Refactoring it would likely be a larger undertaking, as we have also
> said that refs must either have pseudo-ref syntax or start with "refs/".
>
> The sequencer with its "rebase-merge/autostash" files doesn't conform to
> either of those requirements, so we would also have to rename those
> reflike files. I doubt that this is really worth it, so I would keep
> those around as internal implementation details of how the sequencer
> works. These refs are not supposed to be accessed by the user in the
> first place, and we do not document their existence to the best of my
> knowledge. Also, `git rev-parse rebase-merge/autostash` would fail.
Exactly "rebase-merge/autostash" is a detail of the rebase
implementation, it is not a ref that users should be using.
> So overall I think it's fine to leave this internal sequencer state as
> self-contained as it is.
That's my feeling too
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter
From: Phillip Wood @ 2024-01-22 20:13 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: Junio C Hamano
In-Reply-To: <20240119142705.139374-3-karthik.188@gmail.com>
Hi Karthik
On 19/01/2024 14:27, Karthik Nayak wrote:
> The `is_pseudoref_syntax()` function is used to determine if a
> particular refname follows the pseudoref syntax. The pseudoref syntax is
> loosely defined at this instance as all refs which are in caps and use
> underscores. Most of the pseudorefs also have the "HEAD" suffix.
>
> Using this information we make the `is_pseudoref_syntax()` function
> stricter, by adding the check for "HEAD" suffix and for refs which don't
> end with the HEAD suffix, matching them with a predetermined list.
>
> This requires fixing up t1407 to use the "HEAD" suffix for creation of
> pseudorefs.
I'm concerned that this change is a regression. is_pseudoref_syntax() is
used by is_current_worktree_ref() and so scripts that create pseudorefs
that do not conform to your new rules will break as git will no-longer
consider the pseudorefs they create to be worktree specific. The list of
hard coded exceptions also looks quite short, I can see MERGE_AUTOSTASH
and BISECT_START are missing and there are probably others I've not
thought of.
The commit message would be a good place to discuss why you're making
this change, the implications of the change and any alternative
approaches that you considered. As I understand it you're tying to get
round the problem that the files backend stores pseudorefs mixed up with
other non-ref files in $GIT_DIR. Another approach would be to read all
the files whose name matches the pseudoref syntax and see if its
contents looks like a valid ref skipping names like COMMIT_EDITMSG that
we know are not pseudorefs.
Best Wishes
Phillip
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> refs.c | 21 ++++++++++++++++++---
> t/t1407-worktree-ref-store.sh | 12 ++++++------
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 5999605230..b84e173762 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -829,6 +829,14 @@ int is_per_worktree_ref(const char *refname)
>
> int is_pseudoref_syntax(const char *refname)
> {
> + /* TODO: move these pseudorefs to have _HEAD suffix */
> + static const char *const irregular_pseudorefs[] = {
> + "BISECT_EXPECTED_REV",
> + "NOTES_MERGE_PARTIAL",
> + "NOTES_MERGE_REF",
> + "AUTO_MERGE"
> + };
> + size_t i;
> const char *c;
>
> for (c = refname; *c; c++) {
> @@ -837,10 +845,17 @@ int is_pseudoref_syntax(const char *refname)
> }
>
> /*
> - * HEAD is not a pseudoref, but it certainly uses the
> - * pseudoref syntax.
> + * Most pseudorefs end with _HEAD. HEAD itself is not a
> + * pseudoref, but it certainly uses the pseudoref syntax.
> */
> - return 1;
> + if (ends_with(refname, "HEAD"))
> + return 1;
> +
> + for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
> + if (!strcmp(refname, irregular_pseudorefs[i]))
> + return 1;
> +
> + return 0;
> }
>
> static int is_current_worktree_ref(const char *ref) {
> diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
> index 05b1881c59..53592c95f3 100755
> --- a/t/t1407-worktree-ref-store.sh
> +++ b/t/t1407-worktree-ref-store.sh
> @@ -61,18 +61,18 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
> # PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
> # not testing a realistic scenario.
> test_expect_success REFFILES 'for_each_reflog()' '
> - echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
> + echo $ZERO_OID >.git/logs/PSEUDO_MAIN_HEAD &&
> mkdir -p .git/logs/refs/bisect &&
> - echo $ZERO_OID > .git/logs/refs/bisect/random &&
> + echo $ZERO_OID >.git/logs/refs/bisect/random &&
>
> - echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
> + echo $ZERO_OID >.git/worktrees/wt/logs/PSEUDO_WT_HEAD &&
> mkdir -p .git/worktrees/wt/logs/refs/bisect &&
> - echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
> + echo $ZERO_OID >.git/worktrees/wt/logs/refs/bisect/wt-random &&
>
> $RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
> cat >expected <<-\EOF &&
> HEAD 0x1
> - PSEUDO-WT 0x0
> + PSEUDO_WT_HEAD 0x0
> refs/bisect/wt-random 0x0
> refs/heads/main 0x0
> refs/heads/wt-main 0x0
> @@ -82,7 +82,7 @@ test_expect_success REFFILES 'for_each_reflog()' '
> $RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
> cat >expected <<-\EOF &&
> HEAD 0x1
> - PSEUDO-MAIN 0x0
> + PSEUDO_MAIN_HEAD 0x0
> refs/bisect/random 0x0
> refs/heads/main 0x0
> refs/heads/wt-main 0x0
^ permalink raw reply
* [RFC PATCH] editorconfig: limit code lines to 80 characters
From: Carlo Marcelo Arenas Belón @ 2024-01-22 20:14 UTC (permalink / raw)
To: git; +Cc: sandals, Carlo Marcelo Arenas Belón
Since 6f9beef335 (editorconfig: provide editor settings for Git developers,
2018-10-08) a multi editor/IDE configuration file has been provided to help
developers follow Documentation/CodingGuidelines.
The settings are also supposed to mirror what is found in .clang-format, but
the "ColumnLimit: 80" setting wasn't included, so correct that.
---
.editorconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/.editorconfig b/.editorconfig
index f9d819623d..c6d7815c80 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -7,6 +7,7 @@ insert_final_newline = true
[*.{c,h,sh,perl,pl,pm,txt}]
indent_style = tab
tab_width = 8
+max_line_length = 80
[*.py]
indent_style = space
--
2.39.3 (Apple Git-145)
^ permalink raw reply related
* Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs
From: Junio C Hamano @ 2024-01-22 20:16 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git
In-Reply-To: <a7712d26-7c63-4c77-b339-126d05be6b0d@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> ... These refs are not supposed to be accessed by the user in the
>> first place, and we do not document their existence to the best of my
>> knowledge. Also, `git rev-parse rebase-merge/autostash` would fail.
>
> Exactly "rebase-merge/autostash" is a detail of the rebase
> implementation, it is not a ref that users should be using.
>
>> So overall I think it's fine to leave this internal sequencer state as
>> self-contained as it is.
>
> That's my feeling too
Good. As long as "git rev-parse rebase-merge/autostash" fails, and
regardless of the ref backend in use, we always do "read one line from
the on-disk file and run get_oid_hex() on it", I would be happy with
that direction.
Thanks.
[Footnote]
* We seem to overuse strbuf_read_file() even when we expect that
the file is a single-liner. We have read_line_from_git_path() in
wt-status.c that only reads one line and we may want to split it
into two: one that takes a filename and calls the other with
git_path("%s", filename), and the other that takes a path for any
on-disk file, reads a single line, and returns the string that
was read, or something like that. We might later want to update
the "other" function so that it errors out if there are extra
trailing lines (i.e. we expect it is a single-liner, but that
expectation is violated---now what?).
^ permalink raw reply
* Re: [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter
From: Junio C Hamano @ 2024-01-22 20:22 UTC (permalink / raw)
To: Phillip Wood; +Cc: Karthik Nayak, git
In-Reply-To: <ee977173-bc6d-48f6-9bc8-e1d84fe3d95d@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> I'm concerned that this change is a regression. is_pseudoref_syntax()
> is used by is_current_worktree_ref() and so scripts that create
> pseudorefs that do not conform to your new rules will break as git
> will no-longer consider the pseudorefs they create to be worktree
> specific.
Ideally, when the exception list in the function becomes more
complete, those "pseudorefs" created by those scripts shouldn't
probably be created either as common or worktree specific thing
if they are not "pseudoref".
> The list of hard coded exceptions also looks quite short, I
> can see MERGE_AUTOSTASH and BISECT_START are missing and there are
> probably others I've not thought of.
I agree that it is something we need to fix.
> The commit message would be a good place to discuss why you're making
> this change, the implications of the change and any alternative
> approaches that you considered. As I understand it you're tying to get
> round the problem that the files backend stores pseudorefs mixed up
> with other non-ref files in $GIT_DIR.
Yup. The rationale may want to be explained better.
> Another approach would be to
> read all the files whose name matches the pseudoref syntax and see if
> its contents looks like a valid ref skipping names like COMMIT_EDITMSG
> that we know are not pseudorefs.
In the longer term, I'd prefer to see a simpler rule, like "all-caps
or underscore string, ending with _HEAD and nothing else are the
pseudorefs but we have these small number of exceptions that are
grandfathered".
Thanks.
^ permalink raw reply
* Re: [RFC PATCH] editorconfig: limit code lines to 80 characters
From: Junio C Hamano @ 2024-01-22 20:44 UTC (permalink / raw)
To: Carlo Marcelo Arenas Belón; +Cc: git, sandals
In-Reply-To: <20240122201420.72802-1-carenas@gmail.com>
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
> Since 6f9beef335 (editorconfig: provide editor settings for Git developers,
> 2018-10-08) a multi editor/IDE configuration file has been provided to help
> developers follow Documentation/CodingGuidelines.
>
> The settings are also supposed to mirror what is found in .clang-format, but
> the "ColumnLimit: 80" setting wasn't included, so correct that.
> ---
> .editorconfig | 1 +
> 1 file changed, 1 insertion(+)
Sounds quite straight-forward to me.
^ permalink raw reply
* Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Raúl Núñez de Arenas Coronado @ 2024-01-22 20:45 UTC (permalink / raw)
To: git
In-Reply-To: <CAGF1KhWNaO_TUuCPo2L_HzNnR+FnB1Q4H6_xQ2owoH+SnynzEg@mail.gmail.com>
Hi there!
I'm not subscribed to the list, sorry, so I would be more than
grateful if I'm CC'd, but I'll check the mailing list archive from
time to time for replies. Thanks in advance.
It is a common practice for Python virtual environments, pytest and
other tools (not always Python related) to add a '.gitignore' file in
directories these tools create on a repository that should NOT be
committed or tracked. By adding a .gitignore file containing a single
"*", the entire directory is ignored by git. So far, so good.
But when using 'git ls-files --others --exclude-from=<file>', when
<file> is one of those .gitignore files present in a subdir, makes the
command use the patterns in that .gitgnore (in this case, the "*")
against ALL files that would otherwise be listed by using '--others'.
In short: using 'git ls-files --others
--exclude-from=subdir/.gitignore' results in an empty listing if
subdir/.gitignore contains '*". IMHO that pattern should be applied to
the subdir contents and not to the contents of the current directory.
That would be consistent with how git uses .gitignore files in
subfolders.
The obvious solution is to use --exclude-per-directory but it is
deprecated in favor of --exclude-standard and --exclude-standard shows
the same behaviour of --exclude-from=subdir/.gitignore!!!
Is there any way, not using --exclude-per-directory, of listing all
untracked files in the current repository, but ignoring those matching
.gitignore files with the patterns matching the corresponding
subdirectory?
Is this a bug or intended behaviour?
Thanks a lot in advance :)
--
Raúl Núñez de Arenas Coronado
.
^ permalink raw reply
* Re: [PATCH] setup: allow cwd=.git w/ bareRepository=explicit
From: Kyle Lippincott @ 2024-01-22 20:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kyle Lippincott via GitGitGadget, git
In-Reply-To: <xmqqh6j7ej5w.fsf@gitster.g>
On Sat, Jan 20, 2024 at 2:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kyle Lippincott via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Kyle Lippincott <spectral@google.com>
> >
> > The safe.bareRepository setting can be set to 'explicit' to disallow
> > implicit uses of bare repositories, preventing an attack [1] where an
> > artificial and malicious bare repository is embedded in another git
> > repository. Unfortunately, some tooling uses myrepo/.git/ as the cwd
> > when executing commands, and this is blocked when
> > safe.bareRepository=explicit. Blocking is unnecessary, as git already
> > prevents nested .git directories.
>
> In other words, if the directory $D that is the top level of the
> working tree of a non-bare repository, you should be able to chdir
> to "$D/.git" and run your git command without explicitly saying that
> you are inside $GIT_DIR (e.g. "git --git-dir=$(pwd) cmd")?
Correct.
>
> It makes very good sense.
>
> I briefly wondered if this would give us a great usability
> improvement especially for hook scripts, but they are given GIT_DIR
> when called already so that is not a big upside, I guess.
>
> > Teach git to not reject uses of git inside of the .git directory: check
> > if cwd is .git (or a subdirectory of it) and allow it even if
> > safe.bareRepository=explicit.
>
>
> > My primary concern with this patch is that I'm unsure if we need to
> > worry about case-insensitive filesystems (ex: cwd=my_repo/.GIT instead
> > of my_repo/.git, it might not trigger this logic and end up allowed).
>
> You are additionally allowing ".git" so even if somebody has ".GIT"
> that won't be allowed by this change, no?
My concern was what happens if someone on a case-insensitive
filesystem does `cd .GIT` and then attempts to use it. If the cwd path
isn't case-normalized at some point, we'll have a string like
/path/to/my-repo/.GIT from getcwd() and it won't be allowed by this
code, since this code is checking for `.git` in a case sensitive
fashion.
>
> > I'm assuming this isn't a significant concern, for two reasons:
> >
> > * most filesystems/OSes in use today (by number of users) are at least
> > case-preserving, so users/tools will have had to type out .GIT
> > instead of getting it from readdir/wherever.
> > * this is primarily a "quality of life" change to the feature, and if
> > we get it wrong we still fail closed.
>
> Yup.
>
> If we really cared (which I doubt), we could resort to checking with
> is_ntfs_dotgit() and is_hfs_dotgit(), but that would work in the
> direction of loosening the check even further, which I do not know
> is needed.
Agreed, it's not worth the additional complexity.
>
> > - if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT)
> > + if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT &&
> > + !ends_with_path_components(dir->buf, ".git"))
> > return GIT_DIR_DISALLOWED_BARE;
>
> Thanks.
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Junio C Hamano @ 2024-01-22 20:52 UTC (permalink / raw)
To: Raúl Núñez de Arenas Coronado; +Cc: git
In-Reply-To: <CAGF1KhWiYX=3R01Odj2yCNgvx=f5+HRCjRJogWf5eBikuATCcg@mail.gmail.com>
Raúl Núñez de Arenas Coronado <raulnac@gmail.com> writes:
> In short: using 'git ls-files --others
> --exclude-from=subdir/.gitignore' results in an empty listing if
> subdir/.gitignore contains '*". IMHO that pattern should be applied to
> the subdir contents ...
I do not think so.
Imagine what would happen then if you did
$ cp subdir/.gitignore /var/tmp/1
$ git ls-files --others --exclude-from=/var/tmp/1
in such a repository? The "--exclude-from" option is used to name
the contents (set of patterns) that should be used and the path of
the file that happens to contain the contents does not matter. So
you should get the same output as the ls-files command that was told
to use "--exclude-from=subdir/.gitignore".
^ permalink raw reply
* Re: [PATCH 4/4] cherry-pick: Add `--empty` for more robust redundant commit handling
From: Kristoffer Haugsbakk @ 2024-01-22 20:55 UTC (permalink / raw)
To: brianmlyles; +Cc: Taylor Blau, Elijah Newren, git
In-Reply-To: <CAHPHrSddqTEjtfhgVJ=vmRhbtuwXcGEiE1KFZqR1QhKw-HDtSg@mail.gmail.com>
On Sun, Jan 21, 2024, at 19:28, Brian Lyles wrote:
> Thank you -- This will be corrected with v2.
>
> Is the sample pre-commit hook the ideal way to prevent this in the
> future? Or is there some config I could set globally to enforce this
> across repositories? I was having a little trouble finding a good way to
> accomplish this globally.
>
> Thanks,
> Brian Lyles
Oh, and this thread reminded me https://lore.kernel.org/git/xmqqle8hrtcs.fsf@gitster.g/T/#t
that editorconfig[1] has this option:
```
trim_trailing_whitespace = true
```
So I guess that should be enough for all editors that respect this
config (although I haven’t tested it).
🔗 1: https://editorconfig.org/
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Raúl Núñez de Arenas Coronado @ 2024-01-22 21:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqfryprsyp.fsf@gitster.g>
Hi Junio, and thanks a lot for your reply :)
El lun, 22 ene 2024 a las 21:52, Junio C Hamano (<gitster@pobox.com>) escribió:
> Raúl Núñez de Arenas Coronado <raulnac@gmail.com> writes:
>
> > In short: using 'git ls-files --others
> > --exclude-from=subdir/.gitignore' results in an empty listing if
> > subdir/.gitignore contains '*". IMHO that pattern should be applied to
> > the subdir contents ...
>
> I do not think so.
>
> Imagine what would happen then if you did
>
> $ cp subdir/.gitignore /var/tmp/1
> $ git ls-files --others --exclude-from=/var/tmp/1
I understand this use case, yes. I was thinking about what git itself
does when ignoring files, especially when dealing with .gitgnore files
in subdirectories, but clearly this needs a different policy, yes.
What I needed from this command is a way of backing up some ignored
files. These files should not go into the repository, but I'm using
them temporarily for development so it is a good idea to back them up.
I'll just backup the entire repository instead, is not a big deal :))
Again, thanks for your reply and your explanation! It's a huge honor
to be replied to by no less than the Git team leather!
--
Raúl Núñez de Arenas Coronado
.
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Jeff King @ 2024-01-22 21:34 UTC (permalink / raw)
To: Raúl Núñez de Arenas Coronado; +Cc: git
In-Reply-To: <CAGF1KhWiYX=3R01Odj2yCNgvx=f5+HRCjRJogWf5eBikuATCcg@mail.gmail.com>
On Mon, Jan 22, 2024 at 09:45:46PM +0100, Raúl Núñez de Arenas Coronado wrote:
> But when using 'git ls-files --others --exclude-from=<file>', when
> <file> is one of those .gitignore files present in a subdir, makes the
> command use the patterns in that .gitgnore (in this case, the "*")
> against ALL files that would otherwise be listed by using '--others'.
>
> In short: using 'git ls-files --others
> --exclude-from=subdir/.gitignore' results in an empty listing if
> subdir/.gitignore contains '*". IMHO that pattern should be applied to
> the subdir contents and not to the contents of the current directory.
> That would be consistent with how git uses .gitignore files in
> subfolders.
I think Junio covered this with his example, and everything is behaving
as intended (my mental model is that "--exclude-from" is something like
.git/info/exclude or the core.excludesFile option).
But...
> The obvious solution is to use --exclude-per-directory but it is
> deprecated in favor of --exclude-standard and --exclude-standard shows
> the same behaviour of --exclude-from=subdir/.gitignore!!!
...I'm not sure what's going on here. I would think that both
--exclude-standard and --exclude-per-directory would do what you want.
For example, I get:
[setup]
$ git init
$ mkdir subdir
$ echo '*' >subdir/.gitignore
$ git add -f subdir/.gitignore && git commit -m "add gitignore"
$ touch subdir/file file
[no exclusions]
$ git ls-files -o
file
subdir/file
[use .gitignore]
$ git ls-files --exclude-per-directory=.gitignore -o
file
[using standard excludes]
$ git ls-files --exclude-standard -o
file
Do you get different results from that toy repo? If not, then what is
different about your main repo? Do you perhaps have a stray "*" match
somewhere in .git/info/exclude, etc? Or are you still providing
--exclude-from in addition to --exclude-standard?
-Peff
PS I hadn't realized that --exclude-per-directory had been marked as
deprecated. I do agree with e750951e74 (ls-files: guide folks to
--exclude-standard over other --exclude* options, 2023-01-13) in its
goal of guiding people to the easiest option, but I don't know that
there has been any discussion about removing the other ones.
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Junio C Hamano @ 2024-01-22 21:42 UTC (permalink / raw)
To: Raúl Núñez de Arenas Coronado
Cc: git, Elijah Newren, Phillip Wood, Sebastian Thiel
In-Reply-To: <CAGF1KhU=o2rb-tCjZAG278kgoW50NYymsJakUwiMxWTQ33gQYA@mail.gmail.com>
Raúl Núñez de Arenas Coronado <raulnac@gmail.com> writes:
> ... I was thinking about what git itself
> does when ignoring files, especially when dealing with .gitgnore files
> in subdirectories, but clearly this needs a different policy, yes.
What "git" internally does is the equivalent of using the
"--exclude-per-directory" option to honor ".gitignore" in the
subdirectories, and in addition use ".git/info/exclude" as another
source, pretty much the same way as "--exclude-from" reads it.
> What I needed from this command is a way of backing up some ignored
> files. These files should not go into the repository, but I'm using
> them temporarily for development so it is a good idea to back them up.
> I'll just backup the entire repository instead, is not a big deal :))
The current mechanism can be used to list "ignored" files that must
be left out of the project (e.g. the most obvious one being object
files "*.o") but these "ignored" files are considered "expendable"
(i.e. Git is allowed to remove it as needed, e.g., when switching
branches and need to make room---if you have an untracked file F
that is ignored, checking out a branch that has a file F/G requires
the file F to disappear so that a directory can be created there).
We have been discussing to extend the mechanism so that we can have
"precious" files, which also will be left out of the project (e.g.,
"git add ." will not add to the index, just like an ignored file)
but are not considered "expendable". If file F is "precious":
- "git add ." will not add F to the index
- "git status" will not say F is left untracked and can be added
- "git clean -f" will not remove it.
- checking out a branch with a tracked file F/G will *fail*, to
prevent the loss of file.
No code yet, but the design consensus is almost there. Once it
appears, you should be able to say "give me the list of tracked and
precious paths---I do not care about ignored ones that are expendable,
because they can be recreated mechanically and that is why they are
marked ignored", and from such a list of files, you should be able
to drive your back-up solution.
[jc: cc'ed those who are involved in the "precious" discussion].
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Junio C Hamano @ 2024-01-22 21:45 UTC (permalink / raw)
To: Jeff King; +Cc: Raúl Núñez de Arenas Coronado, git
In-Reply-To: <20240122213410.GA811766@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> PS I hadn't realized that --exclude-per-directory had been marked as
> deprecated. I do agree with e750951e74 (ls-files: guide folks to
> --exclude-standard over other --exclude* options, 2023-01-13) in its
> goal of guiding people to the easiest option, but I don't know that
> there has been any discussion about removing the other ones.
I do not think there is any value in _removing_ the perfectly well
working --exclude* options, even though I think --exclude-standard
should be what users and scriptors should be using if they want to
emulate what Git does internally.
^ permalink raw reply
* Re: Fwd: Unexpected behavior of ls-files command when using --others --exclude-from, and a .gitignore file which resides in a subdirectory
From: Jeff King @ 2024-01-22 21:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Elijah Newren, Raúl Núñez de Arenas Coronado, git
In-Reply-To: <xmqq1qa9rqji.fsf@gitster.g>
On Mon, Jan 22, 2024 at 01:45:05PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > PS I hadn't realized that --exclude-per-directory had been marked as
> > deprecated. I do agree with e750951e74 (ls-files: guide folks to
> > --exclude-standard over other --exclude* options, 2023-01-13) in its
> > goal of guiding people to the easiest option, but I don't know that
> > there has been any discussion about removing the other ones.
>
> I do not think there is any value in _removing_ the perfectly well
> working --exclude* options, even though I think --exclude-standard
> should be what users and scriptors should be using if they want to
> emulate what Git does internally.
Yeah, mostly I was surprised that e750951e74 used as strong a word as
"deprecated".
-Peff
^ permalink raw reply
* Re: Git Push Always uses Protocol Version 0
From: Jeff King @ 2024-01-22 22:04 UTC (permalink / raw)
To: Zsolt Imre; +Cc: Junio C Hamano, git
In-Reply-To: <E121C312-5771-47EF-9099-BEC8EFC2B9BD@gmail.com>
On Mon, Jan 22, 2024 at 07:24:53PM +0000, Zsolt Imre wrote:
> And to answer your question of what I cannot do with the "current
> versions" of the protocol: I could do everything, of course. But, if
> there's protocol 0, 1 and 2 and I wanted to implement only version 2,
> I thought I should be able to. If protocol V2 was complete, I would
> not have to worry about implementing V0 and V1 (saving some time and
> headache), especially because I do not care about supporting old
> clients. I may have misunderstood the word "version" and version 2 is
> more of an "extension" to V1?
I think the main confusion is that there simply isn't a "v2" push
protocol. It has not yet been written.
There was discussion when v2 was being worked on there that might be a
single "git serve" endpoint that would handle both fetch and push. But
in practice the backwards-compatibility technique we use requires asking
for the usual "upload-pack" or "receive-pack". And hence there isn't
really a single protocol, but still a fetch protocol which can be v0 or
v2, and a push protocol which is always v0.
It's possible we'd shift direction there, but IMHO there's value in
having separate per-operation endpoints. There's some more discussion
in this sub-thread:
https://lore.kernel.org/git/20181213195305.249059-1-jonathantanmy@google.com/
Now of course a v2 push protocol, if it is ever written, will probably
look a lot like the v2 fetch protocol, and they can probably share a lot
of the implementation. But v0 and v2 are not that different either. In a
hypothetical world where v2 push existed and you could get away with
skipping v0 push entirely, I'd expect that "v2 push and v2 fetch" would
be about the same amount of work as the current "v0 push and v2 fetch".
-Peff
PS I saw some mention of "v1" in this thread; I wasn't sure if this was
meant to refer to "v0" (a mistake I have made lots of times myself).
But if not, "v1" is not really of any interest. It was a brief
experimental phase for the client-upgrade mechanisms, and it behaves
exactly like v0. No version of Git has ever used it by default.
^ permalink raw reply
* Re: [PATCH] ci(github): also skip logs of broken test cases
From: Victoria Dye @ 2024-01-22 22:41 UTC (permalink / raw)
To: Philippe Blain via GitGitGadget, git; +Cc: Johannes Schindelin, Philippe Blain
In-Reply-To: <pull.1649.git.git.1705808313306.gitgitgadget@gmail.com>
Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> When a test fails in the GitHub Actions CI pipeline, we mark it up using
> special GitHub syntax so it stands out when looking at the run log. We
> also mark up "fixed" test cases, and skip passing tests since we want to
> concentrate on the failures.
>
> The finalize_test_case_output function in
> test-lib-github-workflow-markup.sh which performs this markup is however
> missing a fourth case: "broken" tests, i.e. tests using
> 'test_expect_failure' to document a known bug. This leads to these
> "broken" tests appearing along with any failed tests, potentially
> confusing the reader who might not be aware that "broken" is the status
> for 'test_expect_failure' tests that indeed failed, and wondering what
> their commits "broke".
>
> Also skip these "broken" tests so that only failures and fixed tests
> stand out.
>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
> ci(github): also skip logs of broken test cases
>
> * An example of a run with failed tests appearing along with several
> "broken" tests:
> https://github.com/phil-blain/git/actions/runs/7589303055/job/20673657755
> * An example of a run with the same failures, but with this patch on
> top (no "broken" tests listed):
> * https://github.com/phil-blain/git/actions/runs/7598605434/job/20694762480
Thanks for making this change, the more focused logs are much nicer to read
(and ostensibly a bit more performant as well).
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1649%2Fphil-blain%2Fgithub-ci-skip-broken-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1649/phil-blain/github-ci-skip-broken-v1
> Pull-Request: https://github.com/git/git/pull/1649
>
> t/test-lib-github-workflow-markup.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
> index 970c6538cba..33405c90d74 100644
> --- a/t/test-lib-github-workflow-markup.sh
> +++ b/t/test-lib-github-workflow-markup.sh
> @@ -42,8 +42,8 @@ finalize_test_case_output () {
> fixed)
> echo >>$github_markup_output "::notice::fixed: $this_test.$test_count $1"
> ;;
> - ok)
> - # Exit without printing the "ok" tests
> + ok|broken)
> + # Exit without printing the "ok" or ""broken" tests
And the implementation itself is nice and simple. Looks good to me!
> return
> ;;
> esac
>
> base-commit: e02ecfcc534e2021aae29077a958dd11c3897e4c
^ permalink raw reply
* Re: [PATCH v4] tests: move t0009-prio-queue.sh to the new unit testing framework
From: Jeff King @ 2024-01-22 22:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Chandra Pratap via GitGitGadget, git, Chandra Pratap,
Phillip Wood, Chandra Pratap
In-Reply-To: <xmqqwms1tcb0.fsf@gitster.g>
On Mon, Jan 22, 2024 at 11:09:39AM -0800, Junio C Hamano wrote:
> > + case DUMP:
> > + while ((peek = prio_queue_peek(&pq))) {
> > + get = prio_queue_get(&pq);
> > + if (!check(peek == get))
> > + return;
> > + if(!check_int(result[j++], ==, show(get)))
> > + test_msg("failed at result[] index %d", j-1);
> > + }
> > + break;
>
> OK. So this one checks as we go. I am not sure how easy to grok a
> breakage diagnosis with these giving the same message, without
> giving any context of the failure (e.g. when we are fed
>
> INPUT = 6 2 4 GET 5 3 GET GET 1 DUMP
> EXPECT = 2 3 4 1 5 6
>
> and for some reason if the first GET did not yield expected 2 but
> gave us, say, 6, we only see "left: 2, right: 6" followed by "failed
> at result[] index 0", and nothing else.
>
> If it said something like "We pushed 6, 2, 4 and then did GET" to
> give the reader a bit more context, it would make it easier to see
> why we were complaining, i.e. expecting to see 2, instead getting 6.
> But perhaps that is too much to ask to this code?
>
> I dunno. Those who wanted to see an easier-to-diagnose output may
> have better ideas.
I don't have any bright ideas, but here are some dumb ones.
The big issue is that storing and manipulating data in C is tricky and
requires code to do correctly. And the whole idea of these tests is to
avoid having a ton of special code. That's why I actually think that the
existing strategy used in t0009 and elsewhere is pretty good. It pawns
off output handling and such to existing programs.
But if we really want to keep it all in C, some options are:
1. We could log what is happening verbosely to stderr (or maybe stdout
with a TAP "#" prefix?). That lets it go nowhere most of the time,
but you can see it if you are investigating a failure (like the way
our current "-v" works).
Obviously by that same argument you can fire up a debugger to look
at the problem more closely when you have a failure. But failures
aren't always easy to reproduce. Sometimes they're racy, sometimes
CI fails but local runs don't (I've found CI's use of "-v -x"
especially helpful there), etc.
2. We can rely on a dynamic-growth array of ints to store the result,
and then a check_int_array() to compare/output them. That's custom
code for the test harness, but in theory it's reusable across many
tests.
3. In the same direction, we could simply output to a strbuf or
similar, and then compare the strbufs. We could even show a diff
when they are not the same. ;) That is reimplementing what scripts
like t0009 are doing already, but maybe people think there is other
value in shoving it all into a single C program (as I've said, I
remain a bit skeptical).
-Peff
^ permalink raw reply
* Re: [Bug?] "git diff --no-rename A B"
From: Jeff King @ 2024-01-22 23:00 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git
In-Reply-To: <579fd5bc-3bfd-427f-b22d-dab5e7e56eb2@web.de>
On Sat, Jan 20, 2024 at 03:39:38PM +0100, René Scharfe wrote:
> > The issue is that we have "--rename-empty", which of course also
> > provides "--no-rename-empty". And parse-options is happy to let you
> > abbreviate names as long as they are unambiguous. But "--no-rename" _is_
> > ambiguous with "--no-renames". Why don't we catch it?
>
> Because diff_opt_parse() passes PARSE_OPT_KEEP_UNKNOWN_OPT, which makes
> parse_long_opt() skip abbreviation detection. Which it does since
> baa4adc66a (parse-options: disable option abbreviation with
> PARSE_OPT_KEEP_UNKNOWN, 2019-01-27).
OK, it makes sense to me that we should avoid abbreviation entirely with
KEEP_UNKNOWN_OPT, for the reasons given in that commit. But if adding
--rename fixed it, is there another bug lurking? That is, would we do
the wrong thing on a case without KEEP_UNKNOWN_OPT but which had
"--renames" and "--no-rename" defined? Or was it simply the
inconsistency in how KEEP_UNKNOWN_OPT is being applied?
I think it might just be the latter. If I do this:
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index ded8116cc5..e908c7386d 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -124,6 +124,7 @@ int cmd__parse_options(int argc, const char **argv)
struct option options[] = {
OPT_BOOL(0, "yes", &boolean, "get a boolean"),
OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"),
+ OPT_BOOL(0, "do-it", &boolean, "'do' ambiguous with 'doubt'"),
{ OPTION_SET_INT, 'B', "no-fear", &boolean, NULL,
"be brave", PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
OPT_COUNTUP('b', "boolean", &boolean, "increment by one"),
then running:
t/helper/test-tool parse-options --do
correctly complains about the ambiguity (though amusingly it mentions
"--no-no-doubt" in the error message). And if I add KEEP_UNKNOWN_OPT,
then it gives the wrong behavior. But curiously it does so even with
your patch applied. So I think there may be further fixes needed.
-Peff
^ permalink raw reply related
* Re: [PATCH] ci(github): also skip logs of broken test cases
From: Junio C Hamano @ 2024-01-22 23:16 UTC (permalink / raw)
To: Victoria Dye
Cc: Philippe Blain via GitGitGadget, git, Johannes Schindelin,
Philippe Blain
In-Reply-To: <dbe25fff-e1d4-41f2-8f8f-c538e8c2a77e@github.com>
Victoria Dye <vdye@github.com> writes:
> Thanks for making this change, the more focused logs are much nicer to read
> (and ostensibly a bit more performant as well).
> ...
> And the implementation itself is nice and simple. Looks good to me!
Thanks, both.
Queued.
^ permalink raw reply
* Defining a platform support policy (Was: [DISCUSS] Introducing Rust into the Git project)
From: Emily Shaffer @ 2024-01-22 23:17 UTC (permalink / raw)
To: Randall S. Becker
Cc: Taylor Blau, Junio C Hamano, Dragan Simic, Git List,
Johannes Schindelin
In-Reply-To: <007c01da4420$10a7b700$31f72500$@nexbridge.com>
On Wed, Jan 10, 2024 at 3:52 PM <rsbecker@nexbridge.com> wrote:
>
> On Wednesday, January 10, 2024 5:26 PM, Taylor Blau wrote:
> >On Wed, Jan 10, 2024 at 05:15:53PM -0500, rsbecker@nexbridge.com wrote:
> >> Just a brief concern: Rust is not broadly portable. Adding another
> >> dependency to git will remove many existing platforms from future releases.
> >> Please consider this carefully before going down this path.
> >
> >I was hoping to hear from you as one of the few (only?) folks who participate on
> >the list and represent HPE NonStop users.
> >
> >I'm curious which if any of the compiler frontends that I listed in my earlier email
> >would work for you.
>
> Unfortunately, none of the compiler frontends listed previously can be built for NonStop. These appear to all require gcc either directly or transitively, which cannot be ported to NonStop. I do not expect this to change any time soon - and is outside of my control anyway. An attempt was made to port Rust but it did not succeed primarily because of that dependency. Similarly, Golang is also not portable to NonStop because of architecture assumptions made by the Go team that cannot be satisfied on NonStop at this time. If some of the memory/pointer issues are the primary concern, c11 might be something acceptable with smart pointers. C17 will eventually be deployable, but is not available on most currently supported OS versions on the platform.
I hope y'all don't mind me hijacking this part of the thread ;)
But, Randall's remarks bring up something pretty compelling: I don't
think Git has a clearly defined platform support policy. As far as I
can tell, the support policy now is "if you run `make test` on it and
breaks, and you let us know, we'll try to fix it" - without much in
the way of additional caveats. If I look in CodingGuidelines I see a
few "this doesn't work on platform X so don't do it" (like around %z
in printf), but nowhere do I see "how to know if your platform is
supported" or even "here are platforms we have heard Git works OK on".
That causes a lot of confusion for the project - threads like this one
(and presumably a similar one about C99 adoption) become a blend of
"is this change good for the project or not?" and "will this change
leave behind platform X?" that is difficult to pick apart.
Does it make sense for us to formalize a support policy? For example,
if we wanted to formalize the status quo, I could envision:
"""
Platform support: We make a best-effort attempt to solve any bugs
reported to the list, regardless of platform. To prevent breakages in
the first place, consider running Git's `make test` regularly on your
platform and reporting the results to git@vger.kernel.org; or, better
yet, consider adding your platform to the GitHub Actions CI
(configured in `.github/`).
"""
Or, if we wanted to be able to move very nimbly, we could imagine
something much more restrictive (note that I'm not endorsing it, just
illustrating):
"""
Platform support: Git is guaranteed to work well on Linux platforms
using a kernel version that is less than 1 year old. Support for all
other platforms is best-effort; when reporting a bug on another
platform, you may need to patch the issue and verify your fix
yourself.
"""
I suspect there's a happy medium in here somewhere - trying to fix (or
avoid) an issue on a platform which the average developer cannot run
tests on is not a recipe for a happy developer, and a general policy
of "patches welcome" for anything but latest Linux is not a recipe for
happy users.
I see a few axes we can play with:
* which architectures/kernels/OS (do we care about more than the
usual suspects of Linux/Mac/Windows // x86/amd/arm //
POSIX-compliant?)
* age of architectures/kernels (do we care to offer full support for
a 10 or 15 year old OS?)
* new feature compatibility guarantees vs. core
functionality/security fix guarantees (which do we really define
"support" as?)
* test provisioning (do we require a VM we can run CI on, or is a
report generated from a nightly build and mailed to the list OK?)
* test/breakage timing (should the above tests run on every commit to
'next'? every merge to 'master'? every RC?)
* who provides the support (is it the patch author's responsibility
to fix late-breaking platform support bugs? is it the reporter's
responsibility? and especially, how does this interplay with test
provisioning and frequency above?)
If we had clearer answers to these questions, it'd be much simpler to
determine whether experimentation with Rust is possible or useful.
Plus it would make developer lives easier, in general, to understand
how much compatibility support work they're potentially signing up for
when sending a change of any size.
- Emily
^ permalink raw reply
* Re: [PATCH 05/10] sequencer: use the trailer iterator
From: Linus Arver @ 2024-01-22 23:22 UTC (permalink / raw)
To: Linus Arver via GitGitGadget, git
Cc: Emily Shaffer, Junio C Hamano, Christian Couder
In-Reply-To: <fd4a9d54d9522973a4c22e43cb1d7964033d4837.1704869487.git.gitgitgadget@gmail.com>
"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/trailer.h b/trailer.h
> index 50f70556302..d50c9fd79b2 100644
> --- a/trailer.h
> +++ b/trailer.h
> @@ -127,6 +127,19 @@ struct trailer_iterator {
> struct strbuf key;
> struct strbuf val;
>
> + /*
> + * Raw line (e.g., "foo: bar baz") before being parsed as a trailer
> + * key/val pair. This field can contain non-trailer lines because it's
> + * valid for a trailer block to contain such lines (i.e., we only
> + * require 25% of the lines in a trailer block to be trailer lines).
> + */
> + struct strbuf raw;
Originally I used a strbuf here for consistency with the other strbufs
used in the iterator for the key and val members. But now I've realized
that there's no need to make "raw" a strbuf at all, because iterator
users will never need to manipulate the string that this points to. Will
change to just "const char *" in the reroll.
^ permalink raw reply
* Re: [PATCH v2 00/12] Group reffiles tests
From: Junio C Hamano @ 2024-01-23 0:01 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: John Cai via GitGitGadget, git, John Cai
In-Reply-To: <Za5TW-q4cKS8pNNc@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> I've got two minor nits, but other than that this looks good to me. I've
> also verified that all tests continue to pass with the current version
> of the reftable backend.
OK. I've squashed all the nits from you and Karthik into the copy
in my tree. If there is nothing else, let's declare a victory and
merge the topic down to 'next' soonish.
> There's a minor merge conflict with db4192c364 (t: mark tests regarding
> git-pack-refs(1) to be backend specific, 2024-01-10). This conflict
> comes from the fact that both patch series add the REFFILES prereq to
> t3210, semantically the changes are the same. So it doesn't quite matter
> which of both versions we retain as they both do the same.
Yup, that is what I've been resolving them.
Thanks.
^ permalink raw reply
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