Git development
 help / color / mirror / Atom feed
* Re: [DISCUSS] Introducing Rust into the Git project
From: Trevor Gross @ 2024-01-11 23:23 UTC (permalink / raw)
  To: rsbecker; +Cc: brian m. carlson, Taylor Blau, Junio C Hamano, Dragan Simic, git
In-Reply-To: <01c101da44d5$175f1100$461d3300$@nexbridge.com>

On Thu, Jan 11, 2024 at 4:28 PM <rsbecker@nexbridge.com> wrote:
>
> The usable compilers and interpreters on NonStop are c89, c99 (what we use for git), c11, perl, and python3 (for the x86 only). The perl and python do not have sufficient modules to do what would be needed by git. The compilers are invoked using a CLI and are not callable using a library. gcc is, for all intents and purposes, not possible - so anything requiring gcc (for example, Rust), cannot be built.  There is no back-end pluggable component for any of the compilers.
>

Ah, no pluggable backend is unfortunate. Rust only uses GCC to build
the LLVM backend, it isn't actually needed for the language. It does
link libgcc_s for unwinding and I believe some math symbols, but
unwinding can be disabled and other symbols can come from anywhere.

If you can build mrustc (C++ program) [1] then you can use it to
transpile Rust to C. This is how rustc is bootstrapped, and would be
how you bring it up with a different backend on a new platform.

Still, this probably wouldn't be a solution for git.

[1]: https://github.com/thepowersgang/mrustc

^ permalink raw reply

* Re: [PATCH] diffcore-delta: avoid ignoring final 'line' of file
From: Junio C Hamano @ 2024-01-11 23:00 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
In-Reply-To: <pull.1637.git.1705006074626.gitgitgadget@gmail.com>

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/diffcore-delta.c b/diffcore-delta.c
> index c30b56e983b..7136c3dd203 100644
> --- a/diffcore-delta.c
> +++ b/diffcore-delta.c
> @@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r,
>  		n = 0;
>  		accum1 = accum2 = 0;
>  	}
> +	if (n > 0) {
> +		hashval = (accum1 + accum2 * 0x61) % HASHBASE;
> +		hash = add_spanhash(hash, hashval, n);
> +	}

OK, so we were ignoring the final short bit that is not terminated
with LF due to the "continue".  Nicely found.

> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index 85be1367de6..29299acbce7 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -286,4 +286,23 @@ test_expect_success 'basename similarity vs best similarity' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'last line matters too' '
> +	test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
> +	printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
> +	git add nonewline &&
> +	git commit -m "original version of file with no final newline" &&

I found it misleading that the file whose name is nonewline has
bunch of LF including on its last line ;-).

> +	# Change ONLY the first character of the whole file
> +	test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&

Same here, but it is too much to bother rewriting it ...

	{
		test_write_lines ...
		printf ...
	} >incomplete

... like so ("incomplete" stands for "file ending with an incomplete line"),
so I'll let it pass.

> +	printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&


> +	git add nonewline &&
> +	git mv nonewline still-no-newline &&
> +	git commit -a -m "rename nonewline -> still-no-newline" &&
> +	git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
> +	cat >expected <<-\EOF &&
> +	R097	nonewline	still-no-newline

I am not very happy with the hardcoded 97.  You are already using
the non-standard 10% threshold.  If the delta detection that
forgets about the last line is so broken as your proposed log
message noted, shouldn't you be able to construct a sample pair of
preimage and postimage for which the broken version gives so low
similarity to be judged not worth treating as a rename, while the
fixed version gives reasonable similarity to be made into a rename,
by the default threshold?  That way, the test only needs to see if
we got a rename (with any similarity) or a delete and an add.

^ permalink raw reply

* Re: [PATCH] push: improve consistency of output when "up to date"
From: Junio C Hamano @ 2024-01-11 22:33 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Benji Kay via GitGitGadget, git, Benji Kay
In-Reply-To: <CAPig+cRr0V2ecnmxk1H_yF24dwSFA6niPxYXGH0MZ+wGP9m9UA@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

> Thanks. This particular change is proposed periodically...
>
>> diff --git a/transport.c b/transport.c
>> @@ -1467,7 +1467,7 @@ int transport_push(struct repository *r,
>>         else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
>> -               fprintf(stderr, "Everything up-to-date\n");
>> +               fprintf(stderr, "Everything up to date.\n");
>
> ... but has not been considered desirable.
>
> See, for instance, this email thread explaining the rationale for
> avoiding such a change:
> https://lore.kernel.org/git/pull.1298.git.1658908927714.gitgitgadget@gmail.com/T/

Looking at the "grep" hits:

$ git grep -e 'up-to-date.*"' \*.c
builtin/rm.c:	OPT__FORCE(&force, N_("override the up-to-date check"), PARSE_OPT_NOCOMPLETE),
builtin/send-pack.c:		fprintf(stderr, "Everything up-to-date\n");
http-push.c:				fprintf(stderr, "'%s': up-to-date\n", ref->name);
http-push.c:				      "Maybe you are not up-to-date and "
transport.c:		fprintf(stderr, "Everything up-to-date\n");

it is true that these are not marked for translation, which should
be a clue enough that we want them to be exactly the way they are
spelled.  However, they are going to the standard error stream.  Is
it reasonable to expect third-party tools scraping it to find the
string "up-to-date"?

In any case, a safe first step is to add a short comment to each of
these that should not be translated.  Perhaps something along this
line.


------- >8 ------------- >8 ------------- >8 -------
Subject: [PATCH] messages: mark some strings with "up-to-date" not to touch

The treewide clean-up of "up-to-date" strings done in 7560f547
(treewide: correct several "up-to-date" to "up to date", 2017-08-23)
deliberately left some out, but unlike the lines that were changed
by the commit, the lines that were deliberately left untouched by
the commit is impossible to ask "git blame" to link back to the
commit that did not touch them.

Let's do the second best thing, leave a short comment near them, to
make it possible for those who are motivated enough to find out why
we decided to tell them "do not modify".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 builtin/send-pack.c | 1 +
 http-push.c         | 2 ++
 transport.c         | 1 +
 3 files changed, 4 insertions(+)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b7183be970..ac7ec1e643 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -333,6 +333,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!ret && !transport_refs_pushed(remote_refs))
+		/* do not modify this string */
 		fprintf(stderr, "Everything up-to-date\n");
 
 	return ret;
diff --git a/http-push.c b/http-push.c
index b4d0b2a6aa..e4c6645cc2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1851,6 +1851,7 @@ int cmd_main(int argc, const char **argv)
 
 		if (oideq(&ref->old_oid, &ref->peer_ref->new_oid)) {
 			if (push_verbosely)
+				/* do not modify this string */
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
 			if (helper_status)
 				printf("ok %s up to date\n", ref->name);
@@ -1871,6 +1872,7 @@ int cmd_main(int argc, const char **argv)
 				 * commits at the remote end and likely
 				 * we were not up to date to begin with.
 				 */
+				/* do not modify this string */
 				error("remote '%s' is not an ancestor of\n"
 				      "local '%s'.\n"
 				      "Maybe you are not up-to-date and "
diff --git a/transport.c b/transport.c
index bd7899e9bf..c9f39d45f1 100644
--- a/transport.c
+++ b/transport.c
@@ -1467,6 +1467,7 @@ int transport_push(struct repository *r,
 	if (porcelain && !push_ret)
 		puts("Done");
 	else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
+		/* do not modify this string */
 		fprintf(stderr, "Everything up-to-date\n");
 
 done:
-- 
2.43.0-283-ga54a84b333


^ permalink raw reply related

* Re: [PATCH v3] builtin/revert.c: refactor using an enum for cmd-action
From: Junio C Hamano @ 2024-01-11 21:47 UTC (permalink / raw)
  To: Michael Lohmann; +Cc: git, phillip.wood123, phillip.wood, wanja.hentze
In-Reply-To: <20240111200627.64199-1-mi.al.lohmann@gmail.com>

Michael Lohmann <mi.al.lohmann@gmail.com> writes:

> This is done to avoid having to keep the char values in sync in
> different places and also to get compiler warnings on non-exhaustive
> switches.
>
> In the rebase `action` enum there is the enumeration constant
> `ACTION_NONE` which is not particularly descriptive, since it seems to
> imply that no action should be taken. Instead it signals a start of a
> revert/cherry-pick process, so here `ACTION_START` was chosen.
>
> Co-authored-by: Wanja Henze <wanja.hentze@bevuta.com>
> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
> ---
>
> On 11. Jan 2024, at 20:37, Junio C Hamano <gitster@pobox.com> wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>> > I think ACTION_NONE was intended to covey that the user did not pass
>> > one of the OPT_CMDMODE() options like "--continue" as there isn't a
>> > "--start" option. I don't have a strong opinion between "_NONE" and
>> > "_START".
>>
>> I agree with you why NONE is called as such.  If "revert" does not
>> take "--start" (I do not remember offhand), I would think it would
>> be better to follow suit.
> My point was that yes, it might be in sync with what the user passes in
> as arguments, but when I followed the code and saw lots of references to
> ACTION_NONE I was puzzled, since my intuition of that name was that
> _no action_ should be taken (which did not make sense to me).

I know you wrote that ;-).  But _NONE is "no action was specified",
and has been so for a long time in the context of "rebase". I do not
see any confusion expressed there.  I do not expect to see any
confusion here, either, if we were to introduce these new enum.


^ permalink raw reply

* Re: Add support for `git rebase -no-edit`
From: Chaitanya Tata @ 2024-01-11 21:46 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Junio C Hamano, git
In-Reply-To: <ZaBgqlR5F1dv9ttg@nand.local>

On Fri, Jan 12, 2024 at 3:12 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Thu, Jan 11, 2024 at 01:30:55PM -0800, Junio C Hamano wrote:
> > Chaitanya Tata <chaitanya.tk17@gmail.com> writes:
> >
> > > Hi,
> > >
> > > I have a feature request to add `--no-edit` option to `git rebase`
> > > like we do for `git commit`.
> > > The workflow I typically follow is:
> > >
> > > * `git commit -a --fixup=XXX`
> > > * `git rebase  -i HEAD~15 --autosquash`
> > >
> > > But it requires closing the editor without any changes. I can
> > > workaround this using the `GIT_EDITOR` option, see [1]. But it would
> > > be good to have this built-in.
> > >
> > > Thoughts?
> >
> > With what is in the 'master' branch, you do not have to say
> > interactive when you do not want to go interactive.  I.e.
> >
> >     $ git rebase --autosquash HEAD~15
> >
> > should work fine.  Building Git from the source should not be too
> > hard.
Amazing, exactly what I need. And or arg order, I just typed in the command,
I typically use aliases that are in the proper order, but thanks.
>
> Oh, duh. Indeed, 297be59456 (rebase: support --autosquash without -i,
> 2023-11-14) will do what Chaitanya is looking for. I'll give myself pass
> on remembering that patch since it is from last year ;-).
:).

Thanks both for prompt support, I will compile and test this.

^ permalink raw reply

* Re: [PATCH] push: improve consistency of output when "up to date"
From: Eric Sunshine @ 2024-01-11 21:45 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Benji Kay via GitGitGadget, git, Benji Kay
In-Reply-To: <ZaBhHSCT7EOgTo/N@nand.local>

On Thu, Jan 11, 2024 at 4:44 PM Taylor Blau <me@ttaylorr.com> wrote:
> On Thu, Jan 11, 2024 at 09:27:29PM +0000, Benji Kay via GitGitGadget wrote:
> > -             fprintf(stderr, "Everything up-to-date\n");
> > +             fprintf(stderr, "Everything up to date.\n");
>
> Between the two, I have a vague preference towards "up-to-date", which
> would suggest changing the pull command's output to read "Already
> up-to-date". Personally I think that neither of them should include a
> period in their output, but whichever we decide should be done so
> consistently between the two.
>
> Also, should this string be marked for translation?

See: https://lore.kernel.org/git/7n9r5q74-9qr1-29sr-p2n5-943n01s0p78r@tzk.qr/

^ permalink raw reply

* Re: [PATCH] diffcore-delta: avoid ignoring final 'line' of file
From: Taylor Blau @ 2024-01-11 21:45 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
In-Reply-To: <pull.1637.git.1705006074626.gitgitgadget@gmail.com>

On Thu, Jan 11, 2024 at 08:47:54PM +0000, Elijah Newren via GitGitGadget wrote:
> ---
>  diffcore-delta.c       |  4 ++++
>  t/t4001-diff-rename.sh | 19 +++++++++++++++++++
>  2 files changed, 23 insertions(+)

Nice find and fix. This patch LGTM.

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH] push: improve consistency of output when "up to date"
From: Taylor Blau @ 2024-01-11 21:43 UTC (permalink / raw)
  To: Benji Kay via GitGitGadget; +Cc: git, Benji Kay
In-Reply-To: <pull.1638.git.1705008449995.gitgitgadget@gmail.com>

On Thu, Jan 11, 2024 at 09:27:29PM +0000, Benji Kay via GitGitGadget wrote:
> From: okaybenji <okaybenji@gmail.com>
>
> When one issues the pull command, one may see "Already up to date."
> When issuing the push command, one may have seen "Everything up-to-date".
> To improve consistency, "Everything up to date." is printed instead.
> (The hyphens have been removed, and a period has been added.)
>
> Signed-off-by: okaybenji <okaybenji@gmail.com>
> ---
>     push: improve consistency of output when "up to date"
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1638%2Fokaybenji%2Fup-to-date-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1638/okaybenji/up-to-date-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1638
>
>  transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/transport.c b/transport.c
> index bd7899e9bf5..c42cb4e58b4 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1467,7 +1467,7 @@ int transport_push(struct repository *r,
>  	if (porcelain && !push_ret)
>  		puts("Done");
>  	else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
> -		fprintf(stderr, "Everything up-to-date\n");
> +		fprintf(stderr, "Everything up to date.\n");

Between the two, I have a vague preference towards "up-to-date", which
would suggest changing the pull command's output to read "Already
up-to-date". Personally I think that neither of them should include a
period in their output, but whichever we decide should be done so
consistently between the two.

Also, should this string be marked for translation?

Thanks,
Taylor

^ permalink raw reply

* Re: [PATCH] push: improve consistency of output when "up to date"
From: Eric Sunshine @ 2024-01-11 21:43 UTC (permalink / raw)
  To: Benji Kay via GitGitGadget; +Cc: git, Benji Kay
In-Reply-To: <pull.1638.git.1705008449995.gitgitgadget@gmail.com>

On Thu, Jan 11, 2024 at 4:27 PM Benji Kay via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> When one issues the pull command, one may see "Already up to date."
> When issuing the push command, one may have seen "Everything up-to-date".
> To improve consistency, "Everything up to date." is printed instead.
> (The hyphens have been removed, and a period has been added.)
>
> Signed-off-by: okaybenji <okaybenji@gmail.com>

Thanks. This particular change is proposed periodically...

> diff --git a/transport.c b/transport.c
> @@ -1467,7 +1467,7 @@ int transport_push(struct repository *r,
>         else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
> -               fprintf(stderr, "Everything up-to-date\n");
> +               fprintf(stderr, "Everything up to date.\n");

... but has not been considered desirable.

See, for instance, this email thread explaining the rationale for
avoiding such a change:
https://lore.kernel.org/git/pull.1298.git.1658908927714.gitgitgadget@gmail.com/T/

^ permalink raw reply

* would these doc and completion patches like this welcome?
From: Britton Kerin @ 2024-01-11 21:42 UTC (permalink / raw)
  To: git

Someone mentioned that there were other places where <path> should be
<pathspec> and I looked through the docs a bit to see, and also noted
simple completion fixes.

There were a number of small issues I saw that I'll fix if there's
interest but I want to ask before spending time (I have a few patches
submitted at the moment and can't tell if they are unwelcome or what).

To give an idea what I have in mind the rough notes I made on a first
pass through are below.  Note that I haven't got as far as verifying
for sure e.g. what actually takes a pathspec so there may be mistakes
in here.

The question now is is there enough interest that I should proceed or not?


Here's the list:



git-am:

  doc: --exclude and --include probably take <pathspec>, but currently refer
       to <path>.  Note: Underlying git-apply currently says <path-pattern> in
       detailed description, <path> in synopsis

  doc: --directory=<dir> in synopsis and detailed description but should
       probably be <root> since the detailed description in git-am just says
       that "These flags are passed to the git apply"

  comp: --directory= takes and should complete a dir

git-archive:

  doc: is <path> argument really a <path> or should it be <pathspec>?

git-bisect:

  doc: mentions of path in main text should be <pathspec> (assuming
       guy on mailing list who said it actually takes a <pathspec> was right,
       check)

git-clone:

  doc: <directory> or <dir>? who cares I think but elsewhere some use <dir>
       <dir>

  comp: <directory> should be completed

  comp: <git-dir> should be completed

git-describe:

  doc: synopsis mentions <blob> but later text just mentions blob, I think this
       is not how other tools mention such things

git-diff:

  doc: are all the mentions of <path> really <path> or should they be
       <pathspec>?

  doc: is optional arg to --relative really a <path>, or is it a <pathspec>?

  comp: there are lots of options that aren't completed that should be

  comp: NOTE: <path> should not get completed as dir even if they aren't
        <pathspec> since they might be files

  comp: NOTE: the optional arg to --relative should not be completed as an
        actual dir

git-format-patch:

  doc: is optional arg to --relative really a <path>, or is it a <pathspec>?

  doc: what is <boundary>?

  doc: --from in main text but not synopsis

  comp: --to --cc --from don't have optarg completion help for format-patch
        as they do for send-email.  I notice that according to it's
        --git-completion-helper output format-patch doesn't implement --bcc
        hmnm is that really true so weird

  comp: NOTE: the optional arg to --relative should not be completed as an
        actual dir

git-init:

  doc: has both [--template=<template-directory>] and
       [--separate-git-dir <git-dir>], I see no reason one of these should get
       an = and the other not, seems like both should get = and that's how this
       command does it though other git commands seem to doc differently

  doc: <directory> is mentioned in synopsis but only underlined directory in
       body.  elsewhere in same file words like this do get their <> and are
       underlined also.  what is the
       actual git way for this?  Just underline?  Underline and <> (seems
       right to me?)  Underline seems to be the common utility way (for ls, cp,
       ln at least which I checked, but those don't use <> everywhere.  I
       guess I sort of think it should be one or the other not both and git
       uses <> everywhere for args and optargs.  FIXME: sheesh I can't quit
       tell when bold is even getting used in my terminal font it's similar
       enough but actuall bold and <> seems pretty common as well

  comp: arg to --template should be completed as a dir

  comp: arg to --separate-git-dir should be completed as a dir

  comp: the optional [<directory>] arg appears to be the only possible
        non-option argument and so should be easy to complete as a dir

git-log:

  doc: synopsis says <path>... probably it means <pathspec>... check

  doc: full text also mentions <paths> in a number of places, it should
       probably by <pathspec>... but at least <path>... (or sometimes just
       <pathspec> or <path>)

  doc: is optional arg to --relative really a <path>, or is it a <pathspec>?

  comp: NOTE: the optional arg to --relative should not be completed as an
        actual dir

git-pull:

  doc: one option description reads '--squash, --no-squash' while another reads
       '--[no-]verify'.  I think I like the first way better for easy search
       but I think the second might be more common.  Find out the preferred
       way and make consistent.

git-range-diff:

  doc: refers to <path> where it likely means pathspec.  highlights <path>
       but doesn't underline

git-reset:

  doc: says 'can copy the contents of a path out of a commit' I believe it
       really means 'contents of a pathspec' but that's so clunky to read
       and understand and so much less likely to be what someone is attempting
       if they're doing the already somewhat exatic thing described that it's
       probably better to just keep on being wrong here.

git-shortlog:

  doc: refers to <path> but should probably be <pathspec>

  doc: is optional arg to --relative really a <path>, or is it a <pathspec>?

  comp: NOTE: the optional arg to --relative should not be completed as an
        actual dir

  doc: mentions <paths> in one place ('as the <paths>' should be 'as a <path>')
       refers to <path> but should probably be <pathspec>

git-submodule:

  doc: <path> all over the place but then the text mentions <pathspec> in one
       place that looks representative so probably they should all be
       <pathspec>

  doc: describes <path> (an optional argument) under options

git-worktree:

  doc: NOTE: I don't think these should be doced as <pathspec> regardless of
       what the actual code takes.  Out of curiosity what does actual code
       take?

  comp: repair subcommand: <path> should be completed to a real dir in this
        case

gitk:

  doc: says <path> probably means <pathspec>

git-config:

  doc: <file-option> appears in synopsis but isn't mentioned in text

git-reflog:

  doc: takes <log-options>, argubly should be <log-option>...

git-repack:

  comp: --expire-to=<dir> should complete dirs

  comp: --filter-to=<dir> should complete dirs

git-bugreport:

  doc: in general (not just for this command) should opts that need a path
       that's a dir always call it <dir> rather than <path>?

  comp: completion needs implemented

  comp: -o/--ouput-directory should complete dirs

git-diagnose:

  comp: completion needs implemented

  comp: -o/--ouput-directory should complete dirs

git-difftool:

  comp: there are lots of options that need completion added for underlying
        git-diff.  For some reason git difftool --git-completion-help doesn't
        pull in the git-diff options (unlike send-email does format-patch
        options) perhaps this could be fixed?.  Or the git-diff options could
        be factored into a function so they can be used for both _git_diff
        and _git_difftool

git-merge-tree:

  doc: --stdin appears pretty significant, but doesn't have an entry of it's
       own in the OPTIONS section

  comp: not implemented at all yet

git-archimport:

  comp: not implemented, probably not worth it

git-cvsexportcommit:

  comp: not implemented, probably not worth it

git-cvsserver:

  comp: not implemented, probably not worth it

git-quiltimport:

  doc: --patches=<dir> might be better than --patches <dir>, how to handle this
       in general?

  comp: not implemented yet

  comp: --patches should complete dirs

git-send-email:

  doc: send-email docs --to --from --cc with different docs than the
       corresponding format-patch options.  Are they not just passed through in
       the case where format-patch is being done implicitly?  Do they override
       for an already-prepared patch series?  Hmm

  comp: all long opts of send-email and format-patch are accepted via
        the --git-completion-helper mechanism.  Unfortunately this means all
        git-format-patch opts are accepted even when --no-format-patch has been
        given.  Both commands support --git-completion-helper so in theory
        the wrong option set could be filtered back out.  It's not possible
        to do this from the arg since it probably isn't there yet and might
        be ambiguous anyway

    * there are at least some opts for which optargs aren't completed:

  comp: should --reply-to be completed like --to=*|--cc=*|--bcc=*|--from=* are?

  comp: should --envelope-sender be completed like
        --to=*|--cc=*|--bcc=*|--from=* are?

  comp: optarg to --smtp-encryption could be completed

  comp: optarg to --transfer-encoding could be be completed

  comp: optarg to --smtp-debug could be be completed

^ permalink raw reply

* Re: Add support for `git rebase -no-edit`
From: Taylor Blau @ 2024-01-11 21:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chaitanya Tata, git
In-Reply-To: <xmqq4jfjftgw.fsf@gitster.g>

On Thu, Jan 11, 2024 at 01:30:55PM -0800, Junio C Hamano wrote:
> Chaitanya Tata <chaitanya.tk17@gmail.com> writes:
>
> > Hi,
> >
> > I have a feature request to add `--no-edit` option to `git rebase`
> > like we do for `git commit`.
> > The workflow I typically follow is:
> >
> > * `git commit -a --fixup=XXX`
> > * `git rebase  -i HEAD~15 --autosquash`
> >
> > But it requires closing the editor without any changes. I can
> > workaround this using the `GIT_EDITOR` option, see [1]. But it would
> > be good to have this built-in.
> >
> > Thoughts?
>
> With what is in the 'master' branch, you do not have to say
> interactive when you do not want to go interactive.  I.e.
>
>     $ git rebase --autosquash HEAD~15
>
> should work fine.  Building Git from the source should not be too
> hard.

Oh, duh. Indeed, 297be59456 (rebase: support --autosquash without -i,
2023-11-14) will do what Chaitanya is looking for. I'll give myself pass
on remembering that patch since it is from last year ;-).

Thanks,
Taylor

^ permalink raw reply

* Re: Add support for `git rebase -no-edit`
From: Junio C Hamano @ 2024-01-11 21:30 UTC (permalink / raw)
  To: Chaitanya Tata; +Cc: git
In-Reply-To: <CADA7-FOE_81ze8hdaRGLPbipihnvFcEYfp9uwnPxPVxDepG4nA@mail.gmail.com>

Chaitanya Tata <chaitanya.tk17@gmail.com> writes:

> Hi,
>
> I have a feature request to add `--no-edit` option to `git rebase`
> like we do for `git commit`.
> The workflow I typically follow is:
>
> * `git commit -a --fixup=XXX`
> * `git rebase  -i HEAD~15 --autosquash`
>
> But it requires closing the editor without any changes. I can
> workaround this using the `GIT_EDITOR` option, see [1]. But it would
> be good to have this built-in.
>
> Thoughts?

With what is in the 'master' branch, you do not have to say
interactive when you do not want to go interactive.  I.e.

    $ git rebase --autosquash HEAD~15

should work fine.  Building Git from the source should not be too
hard.

By the way, make it a habit to pass non-option argument after dashed
options.  It is easier for your readers to understand your command
line that way.

Thanks.

^ permalink raw reply

* RE: [DISCUSS] Introducing Rust into the Git project
From: rsbecker @ 2024-01-11 21:28 UTC (permalink / raw)
  To: 'Trevor Gross'
  Cc: 'brian m. carlson', 'Taylor Blau',
	'Junio C Hamano', 'Dragan Simic', git
In-Reply-To: <CALNs47vfBH9u9B5B3tWRoEkJJiqne5067A4CFnZ3OaMVvz_gSg@mail.gmail.com>

On Thursday, January 11, 2024 3:08 PM, Trevor Gross wrote:
>On Wed, Jan 10, 2024 at 10:24 PM <rsbecker@nexbridge.com> wrote:
>>
>> There are a number of issues for porting gcc (and Go). The list is fairly long, but the
>summary of what I encountered directly (on the last funded effort of 3) is:
>> 1. There are C syntax constructs required to do anything useful (required for
>access to the OS API) on NonStop that are not in gcc. I can hand code the parser for
>that, but it would take time.
>> 2. The Big Endian x86 architecture is weird to gcc and making that work is not
>easy.
>> 3. There is no assembler on NonStop.
>> 4. The ELF header is very different from standard.
>> 5. The symbol table structure is radically different, so debugging would be (nearly)
>impossible or impractical. gdb was ported to account for the platform differences.
>> 6. The linkage structure is similar but different from standard.
>> 7. The external fixup structure is radically different.
>> 8. The loader does not work the same way, so there are required sections of the
>ELF files on NonStop that are not generated by gcc.
>>
>> There are more, but I just did not get to the point if hitting them. Part of my own
>issue is that I have expertise in parsing and semantic passes of compilers, but my
>code generation skills are not where I want them to be for taking on this effort. Our
>last funded attempt had a code generation expert and he gave up in frustration.
>>
>> If I was hired on to do this, it might have a chance, but at an estimate (not mine)
>of 4-5 person years for a gcc port, best case, my $DAYJOB will not permit it.
>>
>> If gcc could be ported to NonStop, it would solve so many problems. I have heard
>of numerous failed efforts beyond what was officially funded by various companies,
>so this is considered a high-risk project.
>
>Out of curiosity - does the Tandem compiler (assuming that is the correct name)
>have a backend that is usable as a library or via an IR?
>
>If so, maybe it would be possible to write a rustc_codegen_tandem backend like the
>three that exist (rustc_codegen_{llvm,gcc,cranelift}
>at [1]. GCC and cranelift are still under development). This way you sidestep a lot of
>the codegen-specific problems listed above.
>
>I am, of course, not suggesting this as a solution for git and am sure you would
>rather have GCC support. But I wonder how feasible this would be if Rust on
>NonStop is desired at some point.

The usable compilers and interpreters on NonStop are c89, c99 (what we use for git), c11, perl, and python3 (for the x86 only). The perl and python do not have sufficient modules to do what would be needed by git. The compilers are invoked using a CLI and are not callable using a library. gcc is, for all intents and purposes, not possible - so anything requiring gcc (for example, Rust), cannot be built.  There is no back-end pluggable component for any of the compilers.


^ permalink raw reply

* [PATCH] push: improve consistency of output when "up to date"
From: Benji Kay via GitGitGadget @ 2024-01-11 21:27 UTC (permalink / raw)
  To: git; +Cc: Benji Kay, okaybenji

From: okaybenji <okaybenji@gmail.com>

When one issues the pull command, one may see "Already up to date."
When issuing the push command, one may have seen "Everything up-to-date".
To improve consistency, "Everything up to date." is printed instead.
(The hyphens have been removed, and a period has been added.)

Signed-off-by: okaybenji <okaybenji@gmail.com>
---
    push: improve consistency of output when "up to date"

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1638%2Fokaybenji%2Fup-to-date-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1638/okaybenji/up-to-date-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1638

 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.c b/transport.c
index bd7899e9bf5..c42cb4e58b4 100644
--- a/transport.c
+++ b/transport.c
@@ -1467,7 +1467,7 @@ int transport_push(struct repository *r,
 	if (porcelain && !push_ret)
 		puts("Done");
 	else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
-		fprintf(stderr, "Everything up-to-date\n");
+		fprintf(stderr, "Everything up to date.\n");
 
 done:
 	free_refs(local_refs);

base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH] git-p4: stop reaching into the refdb
From: Junio C Hamano @ 2024-01-11 21:20 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <33d6a062ec56be33ed50a42a420be0b023f6f4cf.1704980814.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The git-p4 tool creates a bunch of temporary branches that share a
> common prefix "refs/git-p4-tmp/". These branches get cleaned up via
> git-update-ref(1) after the import has finished. Once done, we try to
> manually remove the now supposedly-empty ".git/refs/git-p4-tmp/"
> directory.
>
> This last step can fail in case there still are any temporary branches
> around that we failed to delete because `os.rmdir()` refuses to delete a
> non-empty directory. It can thus be seen as kind of a sanity check to
> verify that we really did delete all temporary branches.

Wow, thanks for being very careful.  I would just have said "there
is no need for such rmdir---what's a single empty unused directory
between friends, which will be reused later when you run 'git p4'
again?" without thinking things through.

> This is a modification in behaviour for the "files" backend because the
> empty directory does not get deleted anymore. But arguably we should not
> care about such implementation details of the ref backend anyway, and
> this should not cause any user-visible change in behaviour.

Independently, it would be sensible to improve the files backend so
that it removes a subdirectory outside the hierarchies that are
created by refs/files-backend.c:files_init_db() when it becomes
empty (#leftoverbits).

And such a change would also have triggered the error from
os.rmdir(), so this patch is doubly welcomed.

Thanks.




> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  git-p4.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 0eb3bb4c47..3ea1c405e5 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -4251,7 +4251,8 @@ def run(self, args):
>          if self.tempBranches != []:
>              for branch in self.tempBranches:
>                  read_pipe(["git", "update-ref", "-d", branch])
> -            os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
> +            if len(read_pipe(["git", "for-each-ref", self.tempBranchLocation])) > 0:
> +                   die("There are unexpected temporary branches")
>  
>          # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
>          # a convenient shortcut refname "p4".

^ permalink raw reply

* Re: [PATCH 2/2] completion: silence pseudoref existence check
From: Junio C Hamano @ 2024-01-11 21:03 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Stan Hu
In-Reply-To: <24563975fca8df6ae73917e9ee3534933d47c429.1704969119.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> In 44dbb3bf29 (completion: support pseudoref existence checks for
> reftables, 2023-12-19), we have extended the Bash completion script to
> support future ref backends better by using git-rev-parse(1) to check
> for pseudo-ref existence. This conversion has introduced a bug, because
> even though we pass `--quiet` to git-rev-parse(1) it would still output
> the resolved object ID of the ref in question if it exists.
>
> Fix this by redirecting its stdout to `/dev/null` and add a test that
> catches this behaviour. Note that the test passes even without the fix
> for the "files" backend because we parse pseudo refs via the filesystem
> directly in that case. But the test will fail with the "reftable"
> backend.

Thanks.  Will queue.

^ permalink raw reply

* [PATCH] diffcore-delta: avoid ignoring final 'line' of file
From: Elijah Newren via GitGitGadget @ 2024-01-11 20:47 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

hash_chars() would hash lines to integers, and store them in a spanhash,
but cut lines at 64 characters.  Thus, whenever it reached 64 characters
or a newline, it would create a new spanhash.  The problem is, the final
part of the file might not end 64 characters after the previous 'line'
and might not end with a newline.  This could, for example, cause an
85-byte file with 12 lines and only the first character in the file
differing to appear merely 23% similar rather than the expected 97%.
Ensure the last line is included, and add a testcase that would have
caught this problem.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    diffcore-delta: avoid ignoring final 'line' of file
    
    Found while experimenting with converting portions of diffcore-delta to
    Rust.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1637%2Fnewren%2Ffix-diffcore-final-line-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1637/newren/fix-diffcore-final-line-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1637

 diffcore-delta.c       |  4 ++++
 t/t4001-diff-rename.sh | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/diffcore-delta.c b/diffcore-delta.c
index c30b56e983b..7136c3dd203 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r,
 		n = 0;
 		accum1 = accum2 = 0;
 	}
+	if (n > 0) {
+		hashval = (accum1 + accum2 * 0x61) % HASHBASE;
+		hash = add_spanhash(hash, hashval, n);
+	}
 	QSORT(hash->data, (size_t)1ul << hash->alloc_log2, spanhash_cmp);
 	return hash;
 }
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 85be1367de6..29299acbce7 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -286,4 +286,23 @@ test_expect_success 'basename similarity vs best similarity' '
 	test_cmp expected actual
 '
 
+test_expect_success 'last line matters too' '
+	test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
+	printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
+	git add nonewline &&
+	git commit -m "original version of file with no final newline" &&
+
+	# Change ONLY the first character of the whole file
+	test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&
+	printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
+	git add nonewline &&
+	git mv nonewline still-no-newline &&
+	git commit -a -m "rename nonewline -> still-no-newline" &&
+	git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
+	cat >expected <<-\EOF &&
+	R097	nonewline	still-no-newline
+	EOF
+	test_cmp expected actual
+'
+
 test_done

base-commit: 055bb6e9969085777b7fab83e3fee0017654f134
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH 1/2] t9902: verify that completion does not print anything
From: Junio C Hamano @ 2024-01-11 20:29 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Stan Hu
In-Reply-To: <73406ca9c8f38ac2ad8f0e32d6d81f1566a6b4d1.1704969119.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The Bash completion script must not print anything to either stdout or
> stderr. Instead, it is only expected to populate certain variables.
> Tighten our `test_completion ()` test helper to verify this requirement.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t9902-completion.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index aa9a614de3..78cb93bea7 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -87,9 +87,11 @@ test_completion ()
>  	else
>  		sed -e 's/Z$//' |sort >expected
>  	fi &&
> -	run_completion "$1" &&
> +	run_completion "$1" >"$TRASH_DIRECTORY"/output 2>&1 &&

OK, there is a call to test_completion after cd and this is a
reasonable way to make sure we write to a known location.

I would have done the "run_completion should be totally silent"
check immediately after this line, as they logically are more
related to each other than to the two lines that implement "the
'out' file when sorted must match the expectation we just prepared",
but that is not a huge deal.

The filename "output" is something that may tempt folks who add more
tests to this file to use for their own purpose; because we use it
only three times locally here, it might be safer to give it a bit
more specific name, e.g., "run-completion-output" or something.



>  	sort out >out_sorted &&
> -	test_cmp expected out_sorted
> +	test_cmp expected out_sorted &&
> +	test_must_be_empty "$TRASH_DIRECTORY"/output &&
> +	rm "$TRASH_DIRECTORY"/output
>  }
>  
>  # Test __gitcomp.

^ permalink raw reply

* [PATCH v2 2/2] t5541: remove lockfile creation
From: Justin Tobler via GitGitGadget @ 2024-01-11 20:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Patrick Steinhardt, Justin Tobler, Justin Tobler
In-Reply-To: <pull.1634.v2.git.1705004670.gitgitgadget@gmail.com>

From: Justin Tobler <jltobler@gmail.com>

To create error conditions, some tests set up reference locks by
directly creating its lockfile. While this works for the files reference
backend, this approach is incompatible with the reftable backend.
Refactor the test to create a d/f conflict via git-update-ref(1) instead
so that the test is reference backend agnostic.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 t/t5541-http-push-smart.sh | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index df758e187df..9a8bed6c32b 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -232,8 +232,9 @@ test_expect_success 'push --atomic fails on server-side errors' '
 	test_config -C "$d" http.receivepack true &&
 	up="$HTTPD_URL"/smart/atomic-branches.git &&
 
-	# break ref updates for other on the remote site
-	mkdir "$d/refs/heads/other.lock" &&
+	# Create d/f conflict to break ref updates for other on the remote site.
+	git -C "$d" update-ref -d refs/heads/other &&
+	git -C "$d" update-ref refs/heads/other/conflict HEAD &&
 
 	# add the new commit to other
 	git branch -f other collateral &&
@@ -241,18 +242,9 @@ test_expect_success 'push --atomic fails on server-side errors' '
 	# --atomic should cause entire push to be rejected
 	test_must_fail git push --atomic "$up" atomic other 2>output  &&
 
-	# the new branch should not have been created upstream
-	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
-
-	# upstream should still reflect atomic2, the last thing we pushed
-	# successfully
-	git rev-parse atomic2 >expected &&
-	# ...to other.
-	git -C "$d" rev-parse refs/heads/other >actual &&
-	test_cmp expected actual &&
-
-	# the new branch should not have been created upstream
+	# The atomic and other branches should be created upstream.
 	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
+	test_must_fail git -C "$d" show-ref --verify refs/heads/other &&
 
 	# the failed refs should be indicated to the user
 	grep "^ ! .*rejected.* other -> other .*atomic transaction failed" output &&
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v2 1/2] t1401: remove lockfile creation
From: Justin Tobler via GitGitGadget @ 2024-01-11 20:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Patrick Steinhardt, Justin Tobler, Justin Tobler
In-Reply-To: <pull.1634.v2.git.1705004670.gitgitgadget@gmail.com>

From: Justin Tobler <jltobler@gmail.com>

To create error conditions, some tests set up reference locks by
directly creating its lockfile. While this works for the files reference
backend, this approach is incompatible with the reftable backend.
Refactor the test to create a d/f conflict via git-symbolic-ref(1)
instead so that the test is reference backend agnostic.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 t/t1401-symbolic-ref.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index c7745e1bf69..c67e5c134d9 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -106,9 +106,8 @@ test_expect_success LONG_REF 'we can parse long symbolic ref' '
 '
 
 test_expect_success 'symbolic-ref reports failure in exit code' '
-	test_when_finished "rm -f .git/HEAD.lock" &&
-	>.git/HEAD.lock &&
-	test_must_fail git symbolic-ref HEAD refs/heads/whatever
+	# Create d/f conflict to simulate failure.
+	test_must_fail git symbolic-ref refs/heads refs/heads/foo
 '
 
 test_expect_success 'symbolic-ref writes reflog entry' '
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 0/2] Generalize reference locking in tests
From: Justin Tobler via GitGitGadget @ 2024-01-11 20:24 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Patrick Steinhardt, Justin Tobler
In-Reply-To: <pull.1634.git.1704912750.gitgitgadget@gmail.com>

Hello again,

This is the second version my patch series that refactors two tests to no
longer be dependent on the files reference backend. Changes compared to v1:

 * Removed reliance on fifo magic to trigger error conditions.
 * d/f conflicts now used to create errors instead.

Thanks for taking a look!

Justin

Justin Tobler (2):
  t1401: remove lockfile creation
  t5541: remove lockfile creation

 t/t1401-symbolic-ref.sh    |  5 ++---
 t/t5541-http-push-smart.sh | 18 +++++-------------
 2 files changed, 7 insertions(+), 16 deletions(-)


base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1634

Range-diff vs v1:

 1:  cb78b549e5e ! 1:  714f713ccec t1401: generalize reference locking
     @@ Metadata
      Author: Justin Tobler <jltobler@gmail.com>
      
       ## Commit message ##
     -    t1401: generalize reference locking
     +    t1401: remove lockfile creation
      
     -    Some tests set up reference locks by directly creating the lockfile.
     -    While this works for the files reference backend, reftable reference
     -    locks operate differently and are incompatible with this approach.
     -    Refactor the test to use git-update-ref(1) to lock refs instead so that
     -    the test does not need to be aware of how the ref backend locks refs.
     +    To create error conditions, some tests set up reference locks by
     +    directly creating its lockfile. While this works for the files reference
     +    backend, this approach is incompatible with the reftable backend.
     +    Refactor the test to create a d/f conflict via git-symbolic-ref(1)
     +    instead so that the test is reference backend agnostic.
      
          Signed-off-by: Justin Tobler <jltobler@gmail.com>
      
       ## t/t1401-symbolic-ref.sh ##
      @@ t/t1401-symbolic-ref.sh: test_expect_success LONG_REF 'we can parse long symbolic ref' '
     - 	test_cmp expect actual
       '
       
     --test_expect_success 'symbolic-ref reports failure in exit code' '
     + test_expect_success 'symbolic-ref reports failure in exit code' '
      -	test_when_finished "rm -f .git/HEAD.lock" &&
      -	>.git/HEAD.lock &&
      -	test_must_fail git symbolic-ref HEAD refs/heads/whatever
     -+test_expect_success PIPE 'symbolic-ref reports failure in exit code' '
     -+	mkfifo in out &&
     -+	(git update-ref --stdin <in >out &) &&
     -+
     -+	exec 9>in &&
     -+	exec 8<out &&
     -+	test_when_finished "exec 9>&-" &&
     -+	test_when_finished "exec 8<&-" &&
     -+
     -+	echo "start" >&9 &&
     -+	echo "start: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	echo "update HEAD refs/heads/foo" >&9 &&
     -+
     -+	echo "prepare" >&9 &&
     -+	echo "prepare: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	test_must_fail git symbolic-ref HEAD refs/heads/bar
     ++	# Create d/f conflict to simulate failure.
     ++	test_must_fail git symbolic-ref refs/heads refs/heads/foo
       '
       
       test_expect_success 'symbolic-ref writes reflog entry' '
 2:  11fd5091d61 ! 2:  f953a668c6a t5541: generalize reference locking
     @@ Metadata
      Author: Justin Tobler <jltobler@gmail.com>
      
       ## Commit message ##
     -    t5541: generalize reference locking
     +    t5541: remove lockfile creation
      
     -    Some tests set up reference locks by directly creating the lockfile.
     -    While this works for the files reference backend, reftable reference
     -    locks operate differently and are incompatible with this approach.
     -    Generalize reference locking by preparing a reference transaction.
     +    To create error conditions, some tests set up reference locks by
     +    directly creating its lockfile. While this works for the files reference
     +    backend, this approach is incompatible with the reftable backend.
     +    Refactor the test to create a d/f conflict via git-update-ref(1) instead
     +    so that the test is reference backend agnostic.
      
          Signed-off-by: Justin Tobler <jltobler@gmail.com>
      
     @@ t/t5541-http-push-smart.sh: test_expect_success 'push --atomic fails on server-s
       
      -	# break ref updates for other on the remote site
      -	mkdir "$d/refs/heads/other.lock" &&
     -+	mkfifo in out &&
     -+	(git -C "$d" update-ref --stdin <in >out &) &&
     -+
     -+	exec 9>in &&
     -+	exec 8<out &&
     -+	test_when_finished "exec 9>&-" &&
     -+	test_when_finished "exec 8<&-" &&
     -+
     -+	echo "start" >&9 &&
     -+	echo "start: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	echo "update refs/heads/other refs/heads/other" >&9 &&
     -+
     -+	# Prepare reference transaction on `other` reference to lock it and thus
     -+	# break updates on the remote.
     -+	echo "prepare" >&9 &&
     -+	echo "prepare: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     ++	# Create d/f conflict to break ref updates for other on the remote site.
     ++	git -C "$d" update-ref -d refs/heads/other &&
     ++	git -C "$d" update-ref refs/heads/other/conflict HEAD &&
       
       	# add the new commit to other
       	git branch -f other collateral &&
     +@@ t/t5541-http-push-smart.sh: test_expect_success 'push --atomic fails on server-side errors' '
     + 	# --atomic should cause entire push to be rejected
     + 	test_must_fail git push --atomic "$up" atomic other 2>output  &&
     + 
     +-	# the new branch should not have been created upstream
     +-	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
     +-
     +-	# upstream should still reflect atomic2, the last thing we pushed
     +-	# successfully
     +-	git rev-parse atomic2 >expected &&
     +-	# ...to other.
     +-	git -C "$d" rev-parse refs/heads/other >actual &&
     +-	test_cmp expected actual &&
     +-
     +-	# the new branch should not have been created upstream
     ++	# The atomic and other branches should be created upstream.
     + 	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
     ++	test_must_fail git -C "$d" show-ref --verify refs/heads/other &&
     + 
     + 	# the failed refs should be indicated to the user
     + 	grep "^ ! .*rejected.* other -> other .*atomic transaction failed" output &&

-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH 0/2] Generalize reference locking in tests
From: Justin Tobler @ 2024-01-11 20:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Justin Tobler via GitGitGadget, git
In-Reply-To: <xmqqv8807ll1.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> The use of two fifos and avoiding deadlocking parent and child is
> tricky enough that it does feel that it warrants a helper function
> to do the common part of what these two patches add.

To avoid the trickiness that comes with introducing fifos to these
tests, in the next patch version I've followed the suggestion of Peff to
instead create d/f conflicts to create an error condition. It appears
that these tests are not particular to the exact error condition being
triggered and therefore d/f conflicts are much simpler to produce.

Thanks,
Justin

On Wed, Jan 10, 2024 at 6:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Justin Tobler via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This approach is more verbose and may warrant consideration of implementing
> > an abstraction to further simplify this operation. There appears to be one
> > other existing test in t1400 that also follows this pattern. Being that
> > there is only a handful of affected tests the implemented approach may be
> > sufficient as is.
>
> The use of two fifos and avoiding deadlocking parent and child is
> tricky enough that it does feel that it warrants a helper function
> to do the common part of what these two patches add.
>
> I think I read t1401 patch carefully enough to understand what is
> going on, but I cannot yet claim the same for the other one.
>
> Thanks.
>
> > Justin Tobler (2):
> >   t1401: generalize reference locking
> >   t5541: generalize reference locking
> >
> >  t/t1401-symbolic-ref.sh    | 28 ++++++++++++++++++++++++----
> >  t/t5541-http-push-smart.sh | 25 +++++++++++++++++++++++--
> >  2 files changed, 47 insertions(+), 6 deletions(-)
> >
> >
> > base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1634

^ permalink raw reply

* Re: [PATCH 2/2] t5541: generalize reference locking
From: Justin Tobler @ 2024-01-11 20:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Justin Tobler via GitGitGadget, git
In-Reply-To: <xmqqa5pbhfkw.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> On Wed, Jan 10, 2024 at 06:52:30PM +0000, Justin Tobler via GitGitGadget wrote:
>>
>>> From: Justin Tobler <jltobler@gmail.com>
>>>
>>> Some tests set up reference locks by directly creating the lockfile.
>>> While this works for the files reference backend, reftable reference
>>> locks operate differently and are incompatible with this approach.
>>> Generalize reference locking by preparing a reference transaction.
>>
>> As with the first patch, I think we could use d/f conflicts to get the
>> same effect. Perhaps something like this:
>
> Thanks for a great alternative.  I agree that avoiding fifo indeed
> is a better way to go.

For this patch, in the next version, I have also followed Peff's
suggestion to create d/f conflicts to trigger an error condition instead
of using fifos.

Thanks to everyone for the feedback!
Justin


On Thu, Jan 11, 2024 at 12:48 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > On Wed, Jan 10, 2024 at 06:52:30PM +0000, Justin Tobler via GitGitGadget wrote:
> >
> >> From: Justin Tobler <jltobler@gmail.com>
> >>
> >> Some tests set up reference locks by directly creating the lockfile.
> >> While this works for the files reference backend, reftable reference
> >> locks operate differently and are incompatible with this approach.
> >> Generalize reference locking by preparing a reference transaction.
> >
> > As with the first patch, I think we could use d/f conflicts to get the
> > same effect. Perhaps something like this:
>
> Thanks for a great alternative.  I agree that avoiding fifo indeed
> is a better way to go.

^ permalink raw reply

* Re: [PATCH 1/2] t1401: generalize reference locking
From: Justin Tobler @ 2024-01-11 20:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Justin Tobler via GitGitGadget, git
In-Reply-To: <20240111071329.GC48154@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> It looks like you re-create this situation in a backend-agnostic way by
> having two simultaneous updates that conflict on the lock (but don't
> care how that lock is implemented).
>
> That works, but I think we could keep it simple. This test doesn't care
> about the exact error condition we create. The point was just to die in
> create_symref() and make sure the exit code was propagated.

As you mentioned, the original intent was to recreate the same error
condition in a reference backend agnostic manner. Since the test doesn't
care about the exact error condition being used, I agree that creating a
d/f conflict instead is a much simpler and better approach. In the next
patch version I've updated the test in t1401 to use git-symbolic-ref(1)
to produce a d/f error.

Thanks,
Justin


On Thu, Jan 11, 2024 at 1:13 AM Jeff King <peff@peff.net> wrote:
>
> On Wed, Jan 10, 2024 at 06:52:29PM +0000, Justin Tobler via GitGitGadget wrote:
>
> > From: Justin Tobler <jltobler@gmail.com>
> >
> > Some tests set up reference locks by directly creating the lockfile.
> > While this works for the files reference backend, reftable reference
> > locks operate differently and are incompatible with this approach.
> > Refactor the test to use git-update-ref(1) to lock refs instead so that
> > the test does not need to be aware of how the ref backend locks refs.
>
> It looks like you re-create this situation in a backend-agnostic way by
> having two simultaneous updates that conflict on the lock (but don't
> care how that lock is implemented).
>
> That works, but I think we could keep it simple. This test doesn't care
> about the exact error condition we create. The point was just to die in
> create_symref() and make sure the exit code was propagated. So something
> like this would work:
>
>   $ git symbolic-ref refs/heads refs/heads/foo
>   error: unable to write symref for refs/heads: Is a directory
>
> (note that you get a different error message if the refs are packed,
> since there we can notice the d/f conflict manually).
>
> There may be other ways to stimulate a failure. I thought "symbolic-ref
> HEAD refs/heads/.invalid" might work, but sadly the refname format check
> happens earlier.
>
> I think it is worth avoiding the fifo magic if we can. It's complicated,
> and it means that not all platforms run the test.
>
> -Peff

^ permalink raw reply

* Re: [DISCUSS] Introducing Rust into the Git project
From: Trevor Gross @ 2024-01-11 20:07 UTC (permalink / raw)
  To: rsbecker; +Cc: brian m. carlson, Taylor Blau, Junio C Hamano, Dragan Simic, git
In-Reply-To: <00a801da443d$b1539670$13fac350$@nexbridge.com>

On Wed, Jan 10, 2024 at 10:24 PM <rsbecker@nexbridge.com> wrote:
>
> There are a number of issues for porting gcc (and Go). The list is fairly long, but the summary of what I encountered directly (on the last funded effort of 3) is:
> 1. There are C syntax constructs required to do anything useful (required for access to the OS API) on NonStop that are not in gcc. I can hand code the parser for that, but it would take time.
> 2. The Big Endian x86 architecture is weird to gcc and making that work is not easy.
> 3. There is no assembler on NonStop.
> 4. The ELF header is very different from standard.
> 5. The symbol table structure is radically different, so debugging would be (nearly) impossible or impractical. gdb was ported to account for the platform differences.
> 6. The linkage structure is similar but different from standard.
> 7. The external fixup structure is radically different.
> 8. The loader does not work the same way, so there are required sections of the ELF files on NonStop that are not generated by gcc.
>
> There are more, but I just did not get to the point if hitting them. Part of my own issue is that I have expertise in parsing and semantic passes of compilers, but my code generation skills are not where I want them to be for taking on this effort. Our last funded attempt had a code generation expert and he gave up in frustration.
>
> If I was hired on to do this, it might have a chance, but at an estimate (not mine) of 4-5 person years for a gcc port, best case, my $DAYJOB will not permit it.
>
> If gcc could be ported to NonStop, it would solve so many problems. I have heard of numerous failed efforts beyond what was officially funded by various companies, so this is considered a high-risk project.

Out of curiosity - does the Tandem compiler (assuming that is the
correct name) have a backend that is usable as a library or via an IR?

If so, maybe it would be possible to write a rustc_codegen_tandem
backend like the three that exist (rustc_codegen_{llvm,gcc,cranelift}
at [1]. GCC and cranelift are still under development). This way you
sidestep a lot of the codegen-specific problems listed above.

I am, of course, not suggesting this as a solution for git and am sure
you would rather have GCC support. But I wonder how feasible this
would be if Rust on NonStop is desired at some point.

[1]: https://github.com/rust-lang/rust/tree/062e7c6a951c1e4f33c0a6f6761755949cde15ec/compiler

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox