* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Johannes Schindelin @ 2023-10-29 14:14 UTC (permalink / raw)
To: Elijah Newren
Cc: Christian Couder, git, Junio C Hamano, Patrick Steinhardt,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <CABPp-BGCzxL-kpOvZzWRTJcx2v18QHm5ev8bFv7bm0dyNqhKug@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]
Hi Elijah,
On Sat, 28 Oct 2023, Elijah Newren wrote:
> On Tue, Oct 10, 2023 at 5:39 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > * Patch 12/15 (replay: disallow revision specific options and
> > pathspecs) in version 4 has been removed, so there are now only 14
> > patches instead of 15 in the series. This follows a suggestion by
> > Dscho, and goes in the direction Elijah initially wanted before
> > Derrick Stolee argued for disallowing revision specific options and
> > pathspecs.
>
> [... snipping many parts that I agree with...]
>
> > Also instead of forcing reverse order we use the reverse order by
> > default but allow it to be changed using `--reverse`. Thanks to
> > Dscho.
>
> I can see why this might sometimes be useful for exclusively linear
> history, but it seems to open a can of worms and possibly unfixable
> corner cases for non-linear history. I'd rather not do this, or at
> least pull it out of this series and let us discuss it in some follow
> up series. There are some other alternatives that might handle such
> usecases better.
I find myself wishing for an easy way to reverse commits, if only to
switch around the latest two commits while stopped during a rebase.
So it would have been nice for me if there had been an easy, worktree-less
way to make that happen.
I guess this would be going in the direction of reordering commits,
though, something we deliberately left for later?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 0/1] Object ID support for git merge-file
From: Phillip Wood @ 2023-10-29 10:15 UTC (permalink / raw)
To: Elijah Newren, brian m. carlson; +Cc: git, Junio C Hamano
In-Reply-To: <CABPp-BFJn5NsSaaxEMpicqND_-8CBw370kyBQuKHGOX-3PRyMw@mail.gmail.com>
On 29/10/2023 06:24, Elijah Newren wrote:
> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
>> This is in use at
>> GitHub to produce conflict diffs when a merge fails, and it seems
>> generally useful, so I'm sending it here.
>
> But...wouldn't you already have the conflicts generated when doing the
> merge and learning that it fails? Why would you need to generate them
> again?
I was surprised by this as well, but as you say this seems like a
useful addition independent of any specific use at GitHub.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 1/1] merge-file: add an option to process object IDs
From: Phillip Wood @ 2023-10-29 10:12 UTC (permalink / raw)
To: Elijah Newren, brian m. carlson; +Cc: git, Junio C Hamano
In-Reply-To: <CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com>
Hi brian
I agree with everything Elijah said, I just have one more comment
>> @@ -80,12 +88,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
>>
>> fname = prefix_filename(prefix, argv[i]);
>>
>> - if (read_mmfile(mmf, fname))
>> + if (object_id) {
>> + if (repo_get_oid(the_repository, argv[i], &oid))
>> + ret = -1;
It would be nice to print an error message here
ret = error("object '%s' does not exist", argv[i]);
none of the existing error messages are marked for translation so I've
left this untranslated as well.
Best Wishes
Phillip
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Elijah Newren @ 2023-10-29 6:44 UTC (permalink / raw)
To: Sebastian Thiel; +Cc: Junio C Hamano, Josh Triplett, git
In-Reply-To: <918D0772-CDEE-4892-828E-BD8A06C3F1F4@icloud.com>
Hi Sebastian,
On Mon, Oct 23, 2023 at 12:15 AM Sebastian Thiel
<sebastian.thiel@icloud.com> wrote:
>
> On 16 Oct 2023, at 8:02, Sebastian Thiel wrote:
>
> > I don't know if this time will be different as I can only offer to implement
> > the syntax adjustment, whatever that might be (possibly after validating
> > the candidate against a corpus of repositories), along with the update
> > to `git clean` so it leaves precious files alone by default and a new flag
> > to also remove precious files.
>
> I am happy to announce this feature can now be contributed in full by me once
> you give it a go. This would mean that the entirety of `git` would become
> aware of precious files over time.
>
> To my mind, and probably out of ignorance, it seems that once the syntax is
> decided on it's possible for the implementation to start. From there I could
> use Elijah's analysis to know which parts of git to make aware of precious files
> in addition to `git clean`.
>
> I am definitely looking forward to hearing from you :).
So, we typically don't pre-approve patches/features. Junio described
this recently at [1].
However, starting things out with an RFC, as you've done, is certainly
a good first step to gauge whether folks think a feature is useful.
Occasionally, when the feature is bigger or touches lots of areas of
the code, people will even write up a design document, and first get a
review on the document, which then streamlines later reviews since we
have some of the high-level aspects agreed to. Some examples:
* Documentation/technical/hash-function-transition.txt
* Documentation/technical/sparse-checkout.txt
* Documentation/technical/sparse-index.txt
Each of which are in various stages between "these are ideas we think
are good and our plans to get there" to "most of this document has
since been implemented". There are others in that directory too,
though not everything in that directory is a planning document; some
of the files are simply documentation of what already exists.
Anyway, creating a similar planning document and covering the various
cases I mentioned would likely be a very useful next step here. I did
note that multiple ideas have been presented in this thread about the
syntax for specifying precious files, and it'd be good to nail one
down. It would also be nice to see proposed answers to the several
cases I brought up (some of which Junio answered, others of which I
also have potential answers for so I could potentially help you craft
this document, and a few others that someone else would need to fill
in). Sometimes we also want to cover pros/cons of the approaches we
have decided upon, in part because others may come along later and if
they discover a new pro or con that we haven't thought of, then we may
need to rethink the plan.
Hope that helps,
Elijah
[1] https://lore.kernel.org/git/xmqq8r9ommyt.fsf@gitster.g/
^ permalink raw reply
* Re: [PATCH] am, rebase: fix arghelp syntax of --empty
From: Elijah Newren @ 2023-10-29 6:24 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, 徐沛文 (Aleen)
In-Reply-To: <10e09b2d-15d7-4af1-b24c-217f9e2f457a@web.de>
On Sat, Oct 28, 2023 at 4:58 AM René Scharfe <l.s.r@web.de> wrote:
>
> Use parentheses and pipes to present alternatives in the argument help
> for the --empty options of git am and git rebase, like in the rest of
> the documentation.
>
> While at it remove a stray use of the enum empty_action value
> STOP_ON_EMPTY_COMMIT to indicate that no short option is present.
> While it has a value of 0 and thus there is no user-visible change,
> that enum is not meant to hold short option characters. Hard-code 0,
> like we do for other options without a short option.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Documentation/git-rebase.txt | 4 ++--
> builtin/am.c | 2 +-
> builtin/rebase.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..b4526ca246 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -289,7 +289,7 @@ See also INCOMPATIBLE OPTIONS below.
> +
> See also INCOMPATIBLE OPTIONS below.
>
> ---empty={drop,keep,ask}::
> +--empty=(drop|keep|ask)::
> How to handle commits that are not empty to start and are not
> clean cherry-picks of any upstream commit, but which become
> empty after rebasing (because they contain a subset of already
> @@ -695,7 +695,7 @@ be dropped automatically with `--no-keep-empty`).
> Similar to the apply backend, by default the merge backend drops
> commits that become empty unless `-i`/`--interactive` is specified (in
> which case it stops and asks the user what to do). The merge backend
> -also has an `--empty={drop,keep,ask}` option for changing the behavior
> +also has an `--empty=(drop|keep|ask)` option for changing the behavior
> of handling commits that become empty.
>
> Directory rename detection
> diff --git a/builtin/am.c b/builtin/am.c
> index 6655059a57..9bb73d1671 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -2420,7 +2420,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
> { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"),
> N_("GPG-sign commits"),
> PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> - OPT_CALLBACK_F(STOP_ON_EMPTY_COMMIT, "empty", &state.empty_type, "{stop,drop,keep}",
> + OPT_CALLBACK_F(0, "empty", &state.empty_type, "(stop|drop|keep)",
> N_("how to handle empty patches"),
> PARSE_OPT_NONEG, am_option_parse_empty),
> OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index f990811614..ad7baedd4a 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1147,7 +1147,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> "instead of ignoring them"),
> 1, PARSE_OPT_HIDDEN),
> OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
> - OPT_CALLBACK_F(0, "empty", &options, "{drop,keep,ask}",
> + OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|ask)",
> N_("how to handle commits that become empty"),
> PARSE_OPT_NONEG, parse_opt_empty),
> OPT_CALLBACK_F('k', "keep-empty", &options, NULL,
> --
> 2.42.0
Looks good to me; thanks.
^ permalink raw reply
* Re: [PATCH 0/1] Object ID support for git merge-file
From: Elijah Newren @ 2023-10-29 6:24 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano, Phillip Wood
In-Reply-To: <20231024195655.2413191-1-sandals@crustytoothpaste.net>
On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> This series introduces an --object-id option to git merge-file such
> that, instead of reading and writing from files on the system, it reads
> from and writes to the object store using blobs.
This seems like a reasonable capability to want from such a plumbing command.
> This is in use at
> GitHub to produce conflict diffs when a merge fails, and it seems
> generally useful, so I'm sending it here.
But...wouldn't you already have the conflicts generated when doing the
merge and learning that it fails? Why would you need to generate them
again?
(Also, generating them again may risk getting names munged for
conflict markers in edge cases involving renames.)
That said, even if I have questions about your particular usecase, I
think the feature you are submitting here makes sense independently.
I left a few minor questions on the patch itself, but overall it looks
good to me.
^ permalink raw reply
* Re: [PATCH 1/1] merge-file: add an option to process object IDs
From: Elijah Newren @ 2023-10-29 6:17 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano, Phillip Wood
In-Reply-To: <20231024195655.2413191-2-sandals@crustytoothpaste.net>
Hi,
Overall, looks good. Just a couple questions...
On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> From: "brian m. carlson" <bk2204@github.com>
>
[...]
> --- a/Documentation/git-merge-file.txt
> +++ b/Documentation/git-merge-file.txt
> @@ -12,6 +12,9 @@ SYNOPSIS
> 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
> [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
> [--[no-]diff3] <current-file> <base-file> <other-file>
> +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]
> + [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]
> + [--[no-]diff3] <current-oid> <base-oid> <other-oid>
Why was the `[-p|--stdout]` option removed in the second synopsis?
Elsewhere you explicitly call it out as a possibility to be used with
--object-id.
Also, why the extra synopsis instead of just adding a `[--object-id]`
option to the previous one?
[...]
> @@ -80,12 +88,21 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
>
> fname = prefix_filename(prefix, argv[i]);
>
> - if (read_mmfile(mmf, fname))
> + if (object_id) {
> + if (repo_get_oid(the_repository, argv[i], &oid))
> + ret = -1;
> + else if (!oideq(&oid, the_hash_algo->empty_blob))
> + read_mmblob(mmf, &oid);
> + else
> + read_mmfile(mmf, "/dev/null");
Does "/dev/null" have any portability considerations? (I really don't
know; just curious.)
^ permalink raw reply
* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Elijah Newren @ 2023-10-29 6:01 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Christian Couder, git, Junio C Hamano, Patrick Steinhardt,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <bd872b81-80a9-5e4e-dcb6-faebc9671848@gmx.de>
Hi,
On Thu, Oct 26, 2023 at 6:44 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Christian,
>
> On Tue, 10 Oct 2023, Christian Couder wrote:
>
[...]
> > + /* requirements/overrides for revs */
> > -+ revs.reverse = 1;
> > ++ revs.reverse = !revs.reverse;
> > + revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
> > + revs.topo_order = 1;
> > + revs.simplify_history = 0;
>
> This still overrides a couple of command-line options, _silently_. I would
> prefer those three assignments to be moved just before the
> `setup_revisions()` call.
>
> Letting users override these settings may not make much sense, but it
> makes even less sense to pretend to let them override the settings and
> then just ignore them without warning. (See also
> https://en.wikipedia.org/wiki/Principle_of_least_astonishment.)
>
> Moving these three assignments before the `setup_revisions()` call would
> neatly remedy that.
I agree that warnings or error messages would be better.
But if we're talking about something short of that, I'd actually argue
the opposite of what you do here. I intentionally moved these
assignments after setup_revisions(), and in my mind, the purpose in
doing so was to satisfy the Principle of Least Astonishment. My
experience with git-fast-export, where some settings are made before
calling setup_revisions() and then can be overridden, and then do
completely hideous things, was much worse to me than just admitting
the flags are bad given the various assumptions the tool makes. I
have some patches sitting around to fix fast-export that I never got
around to upstreaming, but when it came time to implement git-replay,
I made sure to fix what I viewed as the bigger problem.
[...]
> > @@ Documentation/git-replay.txt (new)
> > +
> > +NAME
> > +----
> > -+git-replay - Replay commits on a different base, without touching working tree
> > ++git-replay - Replay commits on a new base, works on bare repos too
> > +
> > +
> > +SYNOPSIS
>
> As mentioned in
> https://lore.kernel.org/git/03460733-0219-c648-5757-db1958f8042e@gmx.de/,
> I would like the `EXPERIMENTAL` label to be shown prominently here.
> Probably not only the `SYNOPSIS` as I had originally suggested but also in
> the `NAME`.
>
> Otherwise we may end up with the same situation as with the (from my
> perspective, failed) `git switch`/`git restore` experiment, where we
> wanted to explore a better user experience than the overloaded `git
> checkout` command, only to now be stuck with having to maintain
> backward-compatibility for `git switch`/`git restore` command-line options
> that were not meant to be set in stone but to be iterated on, instead. A
> real-life demonstration of [Hyrum's Law](hyrumslaw.com/), if you like. Or,
> from a different angle, we re-enacted https://xkcd.com/927/ in a way.
>
> I'd like to suggest to learn from history and avoid this by tacking on a
> warning label right at the top of the documentation. We may eventually
> manage to iterate `git replay` to a point where it is totally capable to
> supersede `git rebase`, by doing everything the latter does, except
> better, who knows? But we _do_ need the liberty to make sweeping changes
> to this new builtin if we want to have a prayer of doing that. And I fear
> that not even mentioning the EXPERIMENTAL nature right at the top of the
> manual page would just render us into that undesirable corner.
I fully support this. Absolutely, 100%.
Thanks both,
Elijah
^ permalink raw reply
* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Elijah Newren @ 2023-10-29 6:00 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <20231010123847.2777056-1-christian.couder@gmail.com>
Hi,
On Tue, Oct 10, 2023 at 5:39 AM Christian Couder
<christian.couder@gmail.com> wrote:
> * Patch 12/15 (replay: disallow revision specific options and
> pathspecs) in version 4 has been removed, so there are now only 14
> patches instead of 15 in the series. This follows a suggestion by
> Dscho, and goes in the direction Elijah initially wanted before
> Derrick Stolee argued for disallowing revision specific options and
> pathspecs.
That's too strongly worded; and may be misleading. My primary goal in
that discussion was that setup_revisions() should not be a disallowed
API for future consumers such as git-replay. My secondary thought, at
the time, was that although I agreed that setup_revisions() was a
problematic API, I didn't think fixing it should be a prerequisite for
new features to make use of it.
However, your paragraph here could easily be read that I think the
setup_revisions() API is fine. I don't. I actually think fixing the
setup_revisions() API and preventing not only git-replay but many
other commands from accepting non-sensical flags, as suggested by
Stolee, is a very good idea. I even brought up the example
$ git stash show --graph --relative-date --min-parents=3
--simplify-merges --cherry --show-pulls --unpacked -v -t -8
--format=oneline --abbrev=12 --pretty=fuller --show-notes
--encode-email-headers --always --branches --indexed-objects stash@{0}
in the thread[1] along with the comment "guess which flags are ignored
and which aren't". That's garbage. This digging plus various things
Stolee said actually convinced me that perhaps prioritizing fixing the
setup_revisions() API over adding new consumers does make sense.
But, I don't feel nearly as strong about it as Stolee on
prioritization, so I'm not going to object too strongly with this
patch being tossed for now. But I do want to note that I actually
like Stolee's suggestion that we should fix that API and tighten what
many commands accept.
[1] https://lore.kernel.org/git/f5dd91a7-ba11-917a-39e2-2737829558cb@github.com/
> * In patch 2/14 (replay: introduce new builtin) the command is now in
> the "plumbingmanipulators" category instead of the "mainporcelain"
> category. This is more in line with the goal of introducing it as a
> plumbing command for now. Thanks to a suggestion from Dscho.
I do want to eventually make it a porcelain, but I think it's pretty
far from that in its current state, so this is a good change.
> * In patch 6/14 (replay: change rev walking options) the commit
> message has been improved, including its subject, to better match
> and explain what the patch does.
This (and multiple other changes I elided) are good; thanks for
pushing forward on this.
> Also instead of forcing reverse
> order we use the reverse order by default but allow it to be changed
> using `--reverse`. Thanks to Dscho.
I can see why this might sometimes be useful for exclusively linear
history, but it seems to open a can of worms and possibly unfixable
corner cases for non-linear history. I'd rather not do this, or at
least pull it out of this series and let us discuss it in some follow
up series. There are some other alternatives that might handle such
usecases better.
> 6: ec51351889 ! 6: 37d545d5d6 replay: don't simplify history
...
> + We want the command to work from older commits to newer ones by default,
> + but we are Ok with letting users reverse that, using --reverse, if that's
> + what they really want.
As noted above, _I_ am not ok with this yet. Given the patch
prominently bears my name, the "we" here at a minimum is wrong. I
would rather leave this change out for now and discuss it for a
follow-on series.
> @@ Documentation/git-replay.txt (new)
> +
> +NAME
> +----
> -+git-replay - Replay commits on a different base, without touching working tree
> ++git-replay - Replay commits on a new base, works on bare repos too
really minor point: "works on" or "works in" or "works with" ?
^ permalink raw reply
* Re: [Bugs] Hello,I can't build git by ./configure
From: h hm @ 2023-10-29 2:18 UTC (permalink / raw)
To: sunshine@sunshineco.com; +Cc: git@vger.kernel.org, h hm
I use the `gcc conftest.c`,but it's no output
Thanks,but i deleted the git dir and Re-unzip it ,it can build now
Maybe i make clean after make it ,and make configure ,i don't know If this is a bug in git or in make ,or it's not a bug
^ permalink raw reply
* Re: [Bugs] Hello,I can't build git by ./configure
From: Eric Sunshine @ 2023-10-29 1:33 UTC (permalink / raw)
To: h hm; +Cc: git@vger.kernel.org
In-Reply-To: <PSBPR01MB3544370EEA032FA47778135DBDA3A@PSBPR01MB3544.apcprd01.prod.exchangelabs.com>
On Sat, Oct 28, 2023 at 11:39 AM h hm <haoming9245@outlook.com> wrote:
> I want to build commit 2e8e77c in the master
This is a merge commit bringing together multiple topics, so it's
difficult to say which topic may have caused a regression. Perhaps try
git-bisect to narrow the problem down to a single commit.
> configure: Setting lib to 'lib' (the default)
> configure: Will try -pthread then -lpthread to enable POSIX Threads.
> configure: CHECKS for site configuration
> checking for gcc... gcc
> checking whether the C compiler works... no
> configure: error: in `/home/haomi/git':
> configure: error: C compiler cannot create executables
Your config.log shows that it is trying to test-compile this tiny program:
/* confdefs.h */
#define PACKAGE_NAME "git"
#define PACKAGE_TARNAME "git"
#define PACKAGE_VERSION "2.42.GIT"
#define PACKAGE_STRING "git 2.42.GIT"
#define PACKAGE_BUGREPORT "git@vger.kernel.org"
#define PACKAGE_URL ""
/* end confdefs.h. */
int
main (void)
{
;
return 0;
}
with the simplest command `gcc conftest.c`, but it's failing. Perhaps
try compiling that program manually in your `/home/haomi/git'
directory and see what the result is (also check $?).
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Andy Koppe @ 2023-10-28 21:11 UTC (permalink / raw)
To: Oswald Buddenhagen, Liam Beguin; +Cc: Kousik Sanagavarapu, git
In-Reply-To: <ZT0oVKPzVi5TsrdS@ugly>
On 28/10/2023 16:27, Oswald Buddenhagen wrote:
> On Fri, Oct 27, 2023 at 10:20:48PM -0400, Liam Beguin wrote:
>> I agree, %aa isn't the best, I'm definitly opened to suggestions.
>> My preference would've been for something like %ad, but that's already
>> taken.
>>
> H for host would be available. (not to be confused with h for human.)
Both lowercase and uppercase would be needed though, to mirror %ae/%aE
and %al/%aL for choosing whether to respect .mailmap. That actually
rules out the %a@ idea as well.
Andy
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Jacob Stopak @ 2023-10-28 18:05 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <fd54ef08fa676ec12ad6835f0122c4c0@manjaro.org>
On Sat, Oct 28, 2023 at 07:41:47PM +0200, Dragan Simic wrote:
> That's something we can keep thinking about, until we find a good solution.
> Also, a detailed review of the current logic behind displaying the hints
> should be performed first, if you agree.
Yes of course.
> > Yes I agree, that sounds great! Maybe I'll just wait then until seeing
> > your implementation of that before I poke around on mine more. Then I'll
> > apply your patches locally to add my extended option.
>
> Great, thanks. I'll start working on the patches tomorrow or so, and I'll
> get back with any important conclusions or open questions arising from that,
> so we can discuss them further.
:)
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Dragan Simic @ 2023-10-28 17:41 UTC (permalink / raw)
To: Jacob Stopak; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <ZT1GWw886XuXwqlw.jacob@initialcommit.io>
On 2023-10-28 19:35, Jacob Stopak wrote:
> On Sat, Oct 28, 2023 at 06:20:41PM +0200, Dragan Simic wrote:
>> I agree, there should be a well-placed hint, but we'd need to think
>> really
>> well where to place it, so we don't annoy experienced git users too
>> much,
>> while we also inform the less experienced users.
>
> Yes, hmm, I wonder if maybe we could add the hint for the extended
> option
> only in the case that the user uses the --verbose option either on the
> command line or via the config setting. Since using the verbose option
> tells us the user is asking for more details, that might be a good time
> to inform them of the _even more detailed_ option, but of course that
> hint could be disabled easily if they preferred the "basic" verbosity.
That's something we can keep thinking about, until we find a good
solution. Also, a detailed review of the current logic behind
displaying the hints should be performed first, if you agree.
>> Sounds good, thank you. If you agree, I'll go ahead and implement
>> support
>> for a few "<command>.verbose" configuration options during the next
>> week or
>> so, and submit the patches. I'll most probably come to some more
>> important
>> conclusions while implementing that, which I'll relay over, of course.
>
> Yes I agree, that sounds great! Maybe I'll just wait then until seeing
> your implementation of that before I poke around on mine more. Then
> I'll
> apply your patches locally to add my extended option.
Great, thanks. I'll start working on the patches tomorrow or so, and
I'll get back with any important conclusions or open questions arising
from that, so we can discuss them further.
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Jacob Stopak @ 2023-10-28 17:35 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <37e7bd8f6f4b75aa3b31dc98804b1334@manjaro.org>
On Sat, Oct 28, 2023 at 06:20:41PM +0200, Dragan Simic wrote:
> > Hehe ok, maybe there is room in some hints to notify users of the
> > extended option...
>
> I agree, there should be a well-placed hint, but we'd need to think really
> well where to place it, so we don't annoy experienced git users too much,
> while we also inform the less experienced users.
Yes, hmm, I wonder if maybe we could add the hint for the extended option
only in the case that the user uses the --verbose option either on the
command line or via the config setting. Since using the verbose option
tells us the user is asking for more details, that might be a good time
to inform them of the _even more detailed_ option, but of course that
hint could be disabled easily if they preferred the "basic" verbosity.
> > > > Any thoughts on what the section in the config for a more general
> > > > setting like this might be named?
> > >
> > > Maybe "core.verbose"? We already have "core.pager", which kind of
> > > affects the way all command outputs look like.
> >
> > Ok! The idea of using "core" came to mind but I wasn't sure if that was
> > more for lower-level settings or more general things.
>
> I also considered the "core" section to be reserved for the very low-level
> internal things, but having "core.pager" clearly allows other appropriate
> configuration options to be placed here.
Ok awesome!
> > Great. Thanks a lot for all the feedback. Let me doctor up the patch
> > series to take these things into account and submit an RFC v3 :D
>
> Sounds good, thank you. If you agree, I'll go ahead and implement support
> for a few "<command>.verbose" configuration options during the next week or
> so, and submit the patches. I'll most probably come to some more important
> conclusions while implementing that, which I'll relay over, of course.
Yes I agree, that sounds great! Maybe I'll just wait then until seeing
your implementation of that before I poke around on mine more. Then I'll
apply your patches locally to add my extended option.
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Dragan Simic @ 2023-10-28 16:20 UTC (permalink / raw)
To: Jacob Stopak; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <ZT0m68HWZS/tDGtH.jacob@initialcommit.io>
On 2023-10-28 17:21, Jacob Stopak wrote:
> On Sat, Oct 28, 2023 at 07:55:31AM +0200, Dragan Simic wrote:
>>> Basically what I'm asking is if commands that already have a
>>> --verbose
>>> flag would just get a config setting that does the existing thing by
>>> default?
>>
>> Well, not by default. The default values would remain "false", so
>> nothing
>> jumps out of nowhere.
>
> Right, sorry, I worded that poorly.
No worries, just wanted to make sure we're on the same page.
>> Yes, that's what I propose. It also looks like a logical choice to
>> me, and
>> it would leave space for some possible later changes to the
>> "<command>.verbose = extended" verbosity, without tying it to the
>> tables.
>> We'd also leave some space that way for even maybe an additional level
>> of
>> verbosity, be it "<command>.verbose = simple", "<command>.verbose =
>> graphical" or whatever.
>>
>> Perhaps this scheme should also support "<command>.verbose = basic",
>> which
>> would be an alias for "<command>.verbose = true", for additional
>> clarity.
>>
> Sounds good!
I'm glad you agree.
>> Perhaps it would also be good to nudge the newbies a bit by requesting
>> them
>> to enable the extended verbosity for each command by hand. That way
>> they
>> would both learn a bit about the way git configuration works, which
>> they
>> ultimately can't escape from, and they would be excited to learn new
>> git
>> commands. Or I at least hope so. :)
>>
> Hehe ok, maybe there is room in some hints to notify users of the
> extended option...
I agree, there should be a well-placed hint, but we'd need to think
really well where to place it, so we don't annoy experienced git users
too much, while we also inform the less experienced users.
>> > Any thoughts on what the section in the config for a more general
>> > setting like this might be named?
>>
>> Maybe "core.verbose"? We already have "core.pager", which kind of
>> affects
>> the way all command outputs look like.
>
> Ok! The idea of using "core" came to mind but I wasn't sure if that was
> more for lower-level settings or more general things.
I also considered the "core" section to be reserved for the very
low-level internal things, but having "core.pager" clearly allows other
appropriate configuration options to be placed here.
> Great. Thanks a lot for all the feedback. Let me doctor up the patch
> series to take these things into account and submit an RFC v3 :D
Sounds good, thank you. If you agree, I'll go ahead and implement
support for a few "<command>.verbose" configuration options during the
next week or so, and submit the patches. I'll most probably come to
some more important conclusions while implementing that, which I'll
relay over, of course.
^ permalink raw reply
* [Bugs] Hello,I can't build git by ./configure
From: h hm @ 2023-10-28 15:39 UTC (permalink / raw)
To: git@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 521 bytes --]
I want to build commit 2e8e77c in the master
Step:
cd git
make configure
./configure
Out:
configure: Setting lib to 'lib' (the default)
configure: Will try -pthread then -lpthread to enable POSIX Threads.
configure: CHECKS for site configuration
checking for gcc... gcc
checking whether the C compiler works... no
configure: error: in `/home/haomi/git':
configure: error: C compiler cannot create executables
See `config.log' for more details
But before that commit ,build ok
There is my config.log
[-- Attachment #1.2: Type: text/html, Size: 2696 bytes --]
[-- Attachment #2: config.log --]
[-- Type: application/octet-stream, Size: 6380 bytes --]
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.
It was created by git configure 2.42.GIT, which was
generated by GNU Autoconf 2.71. Invocation command line was
$ ./configure prefix=/usr/local/git
## --------- ##
## Platform. ##
## --------- ##
hostname = DESKTOP-4TTPHMR
uname -m = x86_64
uname -r = 3.4.9-1.x86_64
uname -s = CYGWIN_NT-10.0-25982
uname -v = 2023-09-06 11:19 UTC
/usr/bin/uname -p = unknown
/bin/uname -X = unknown
/bin/arch = x86_64
/usr/bin/arch -k = unknown
/usr/convex/getsysinfo = unknown
/usr/bin/hostinfo = unknown
/bin/machine = unknown
/usr/bin/oslevel = unknown
/bin/universe = unknown
PATH: /usr/local/bin/
PATH: /usr/bin/
PATH: /cygdrive/c/WINDOWS/system32/
PATH: /cygdrive/c/WINDOWS/
PATH: /cygdrive/c/WINDOWS/System32/Wbem/
PATH: /cygdrive/c/WINDOWS/System32/WindowsPowerShell/v1.0/
PATH: /cygdrive/c/WINDOWS/System32/OpenSSH/
PATH: /cygdrive/c/Program Files/dotnet/
PATH: /cygdrive/c/Program Files/Bandizip/
PATH: /cygdrive/c/Users/haomi/AppData/Local/Microsoft/WindowsApps/
## ----------- ##
## Core tests. ##
## ----------- ##
configure:2628: Setting lib to 'lib' (the default)
configure:2652: Will try -pthread then -lpthread to enable POSIX Threads.
configure:2688: CHECKS for site configuration
configure:2842: checking for gcc
configure:2863: found /usr/bin/gcc
configure:2874: result: gcc
configure:3227: checking for C compiler version
configure:3236: gcc --version >&5
gcc (GCC) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
configure:3247: $? = 0
configure:3236: gcc -v >&5
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-cygwin/11/lto-wrapper.exe
Target: x86_64-pc-cygwin
Configured with: /mnt/share/cygpkgs/gcc/gcc.x86_64/src/gcc-11.4.0/configure --srcdir=/mnt/share/cygpkgs/gcc/gcc.x86_64/src/gcc-11.4.0 --prefix=/usr --exec-prefix=/usr --localstatedir=/var --sysconfdir=/etc --docdir=/usr/share/doc/gcc --htmldir=/usr/share/doc/gcc/html -C --build=x86_64-pc-cygwin --host=x86_64-pc-cygwin --target=x86_64-pc-cygwin --without-libiconv-prefix --without-libintl-prefix --libexecdir=/usr/lib --with-gcc-major-version-only --enable-shared --enable-shared-libgcc --enable-static --enable-version-specific-runtime-libs --enable-bootstrap --enable-__cxa_atexit --with-dwarf2 --with-tune=generic --enable-languages=ada,c,c++,d,fortran,lto,objc,obj-c++,jit --enable-graphite --enable-threads=posix --enable-libatomic --enable-libgomp --enable-libquadmath --enable-libquadmath-support --disable-libssp --enable-libada --disable-symvers --disable-multilib --with-gnu-ld --with-gnu-as --with-cloog-include=/usr/include/cloog-isl --without-libiconv-prefix --without-libintl-prefix --with-system-zlib --enable-linker-build-id --with-default-libstdcxx-abi=gcc4-compatible --enable-libstdcxx-filesystem-ts
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.4.0 (GCC)
configure:3247: $? = 0
configure:3236: gcc -V >&5
gcc: error: unrecognized command-line option '-V'
gcc: fatal error: no input files
compilation terminated.
configure:3247: $? = 1
configure:3236: gcc -qversion >&5
gcc: error: unrecognized command-line option '-qversion'; did you mean '--version'?
gcc: fatal error: no input files
compilation terminated.
configure:3247: $? = 1
configure:3236: gcc -version >&5
gcc: error: unrecognized command-line option '-version'
gcc: fatal error: no input files
compilation terminated.
configure:3247: $? = 1
configure:3267: checking whether the C compiler works
configure:3289: gcc conftest.c >&5
configure:3293: $? = 1
configure:3333: result: no
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "git"
| #define PACKAGE_TARNAME "git"
| #define PACKAGE_VERSION "2.42.GIT"
| #define PACKAGE_STRING "git 2.42.GIT"
| #define PACKAGE_BUGREPORT "git@vger.kernel.org"
| #define PACKAGE_URL ""
| /* end confdefs.h. */
|
| int
| main (void)
| {
|
| ;
| return 0;
| }
configure:3338: error: in `/home/haomi/git':
configure:3340: error: C compiler cannot create executables
See `config.log' for more details
## ---------------- ##
## Cache variables. ##
## ---------------- ##
ac_cv_env_CC_set=
ac_cv_env_CC_value=
ac_cv_env_CFLAGS_set=set
ac_cv_env_CFLAGS_value=
ac_cv_env_CPPFLAGS_set=
ac_cv_env_CPPFLAGS_value=
ac_cv_env_CPP_set=
ac_cv_env_CPP_value=
ac_cv_env_LDFLAGS_set=
ac_cv_env_LDFLAGS_value=
ac_cv_env_LIBS_set=set
ac_cv_env_LIBS_value=
ac_cv_env_build_alias_set=
ac_cv_env_build_alias_value=
ac_cv_env_host_alias_set=
ac_cv_env_host_alias_value=
ac_cv_env_target_alias_set=
ac_cv_env_target_alias_value=
ac_cv_prog_ac_ct_CC=gcc
## ----------------- ##
## Output variables. ##
## ----------------- ##
ALLOCA=''
AR=''
ASCIIDOC=''
CC='gcc'
CFLAGS=''
CPP=''
CPPFLAGS=''
CURL_CONFIG=''
DEFS=''
DIFF=''
ECHO_C=''
ECHO_N='-n'
ECHO_T=''
EGREP=''
EXEEXT=''
GREP=''
LDFLAGS=''
LIBOBJS=''
LIBS=''
LTLIBOBJS=''
OBJEXT=''
PACKAGE_BUGREPORT='git@vger.kernel.org'
PACKAGE_NAME='git'
PACKAGE_STRING='git 2.42.GIT'
PACKAGE_TARNAME='git'
PACKAGE_URL=''
PACKAGE_VERSION='2.42.GIT'
PATH_SEPARATOR=':'
SHELL='/bin/sh'
TAR=''
TCLTK_PATH=''
ac_ct_AR=''
ac_ct_CC='gcc'
bindir='${exec_prefix}/bin'
build_alias=''
datadir='${datarootdir}'
datarootdir='${prefix}/share'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
dvidir='${docdir}'
exec_prefix='NONE'
host_alias=''
htmldir='${docdir}'
includedir='${prefix}/include'
infodir='${datarootdir}/info'
libdir='${exec_prefix}/lib'
libexecdir='${exec_prefix}/libexec'
localedir='${datarootdir}/locale'
localstatedir='${prefix}/var'
mandir='${datarootdir}/man'
oldincludedir='/usr/include'
pdfdir='${docdir}'
prefix='/usr/local/git'
program_transform_name='s,x,x,'
psdir='${docdir}'
runstatedir='${localstatedir}/run'
sbindir='${exec_prefix}/sbin'
sharedstatedir='${prefix}/com'
sysconfdir='${prefix}/etc'
target_alias=''
## ----------- ##
## confdefs.h. ##
## ----------- ##
/* confdefs.h */
#define PACKAGE_NAME "git"
#define PACKAGE_TARNAME "git"
#define PACKAGE_VERSION "2.42.GIT"
#define PACKAGE_STRING "git 2.42.GIT"
#define PACKAGE_BUGREPORT "git@vger.kernel.org"
#define PACKAGE_URL ""
configure: exit 77
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Oswald Buddenhagen @ 2023-10-28 15:27 UTC (permalink / raw)
To: Liam Beguin; +Cc: Kousik Sanagavarapu, git
In-Reply-To: <20231028022048.GA1784118@shaak>
On Fri, Oct 27, 2023 at 10:20:48PM -0400, Liam Beguin wrote:
>I agree, %aa isn't the best, I'm definitly opened to suggestions.
>My preference would've been for something like %ad, but that's already
>taken.
>
H for host would be available. (not to be confused with h for human.)
in retrospect i'd say that it was unwise to use separate letters for the
various forms of dates - a set of qualifiers that would be applied to a
single specifier would be nicer. ah well ...
regards
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] Noobify format for status, add, restore
From: Jacob Stopak @ 2023-10-28 15:21 UTC (permalink / raw)
To: Dragan Simic; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <2a0ba4c8e96cb7d2ea66dd1e78cdd39c@manjaro.org>
On Sat, Oct 28, 2023 at 07:55:31AM +0200, Dragan Simic wrote:
> > So I assume an "add.verbose" config option would just always print that
> > without having to specify the -v, --verbose flag when running the
> > command?
>
> Yes, that's how I see it. Setting "add.verbose" to "true", to be precise,
> or to "basic", which I'll explain a bit further later in my response.
Ok, gotcha!
> > Basically what I'm asking is if commands that already have a --verbose
> > flag
> > would just get a config setting that does the existing thing by default?
>
> Well, not by default. The default values would remain "false", so nothing
> jumps out of nowhere.
Right, sorry, I worded that poorly.
> > > > So it seems like currently "verbose" is used for various things among
> > > > the command set...
> > >
> > > Yes, that's the basic verbosity, as I named it above.
Ok.
> > Would it make sense to try to define a more consistent type of output or
> > format style for "verbosity" across different commands? As it stands it
> > seems each command treats verbosity in its own way which makes it hard
> > to interpret exactly what it will do each time...
>
> We'd have to follow the already established behavior of the commands, and
> there are the man pages to describe what's going on with the verbosity for
> each command. In other words, nothing would get changed, just some more
> knobs would be added, for those who prefer to have the additional verbosity
> enabled.
Ok I see.
> > Ok so it sounds like you prefer to use "verbose" as the setting key?
> > I guess at this point that might make more sense since commit.verbose
> > already exists, and existing options could be packed into it like you
> > said instead of just true or false.
>
> It looks like a logical choice to me.
>
Ok.
> > And then my thing here would just be called "command.verbose =
> > extended"?
>
> Yes, that's what I propose. It also looks like a logical choice to me, and
> it would leave space for some possible later changes to the
> "<command>.verbose = extended" verbosity, without tying it to the tables.
> We'd also leave some space that way for even maybe an additional level of
> verbosity, be it "<command>.verbose = simple", "<command>.verbose =
> graphical" or whatever.
>
> Perhaps this scheme should also support "<command>.verbose = basic", which
> would be an alias for "<command>.verbose = true", for additional clarity.
>
Sounds good!
>
> Perhaps it would also be good to nudge the newbies a bit by requesting them
> to enable the extended verbosity for each command by hand. That way they
> would both learn a bit about the way git configuration works, which they
> ultimately can't escape from, and they would be excited to learn new git
> commands. Or I at least hope so. :)
>
Hehe ok, maybe there is room in some hints to notify users of the
extended option...
> > And it's true that some users might only want the extended (or any
> > format) for specific commands. I think a happy medium then is to have
> > the command-specific settings like you mention, plus one toplevel
> > option that enables a specific type of output format among all commands
> > (and overrides the command-specific settings), so that the user can
> > choose which they prefer.
>
> That's something we can consider as an additional configuration option.
> That way, users could also enable the basic verbosity for all commands,
> which may also be usable.
>
Cool!
> > Any thoughts on what the section in the config for a more general
> > setting like this might be named?
>
> Maybe "core.verbose"? We already have "core.pager", which kind of affects
> the way all command outputs look like.
Ok! The idea of using "core" came to mind but I wasn't sure if that was
more for lower-level settings or more general things.
Great. Thanks a lot for all the feedback. Let me doctor up the patch
series to take these things into account and submit an RFC v3 :D
^ permalink raw reply
* Re: [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Isoken Ibizugbe @ 2023-10-28 14:07 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD1+aWGymjssk5CotPjEmhu5sMcTy-b7eJc4fw-UA41Qig@mail.gmail.com>
On Sat, Oct 28, 2023 at 1:38 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Sat, Oct 28, 2023 at 12:41 PM Isoken Ibizugbe <isokenjune@gmail.com> wrote:
> >
> > On Sat, Oct 28, 2023 at 9:07 AM Christian Couder
> > <christian.couder@gmail.com> wrote:
>
> > > > #define DIGIT "0123456789"
> > > >
> > > > static void t_digit_type(void)
> > > > {
> > > > int i;
> > > > const char *digits = DIGIT;
> > > > for (i = 0; digits[i]; i++)
> > > > {
> > > > check_int(isdigit(digits[i]), ==, 0);
> > > > }
> > >
> > > This tests that isdigit() returns 0 for each of the characters in
> > > "0123456789", but first I think isdigit() should return 1, not 0 for
> > > those characters.
> >
> > yes, that is true. should I send a re-roll?
>
> Yes, please.
#include "test-lib.h"
#include "ctype.h"
static void t_digit_type(void)
{
int i;
for (i = 0; i < 256; i++)
{
if (i < '0' || i > '9')
check_int(isdigit(i), ==, 0);
else
check_int(isdigit(i), ==, 1);
}
}
int main(void)
{
TEST(t_digit_type(), "Character is a digit");
return test_done();
}
^ permalink raw reply
* Re: [OUTREACHY] Final Application For Git Internshhip
From: Naomi Ibe @ 2023-10-28 13:17 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD34gre7Xxw=kFop93tLsGXc3RxasstBWpa6RaJ6VyEpFQ@mail.gmail.com>
On Sat, Oct 28, 2023 at 9:25 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Oct 25, 2023 at 2:14 PM Naomi Ibe <naomi.ibeh69@gmail.com> wrote:
> >>
> >> It could help to say if your contribution has been merged to 'master',
> >> 'next', 'seen' or not at all.
> >
> > My microproject contribution has been scheduled to be merged to the master branch.
>
> Great, please add this information to your application.
>
> >> I think that one of the important tasks to be done early is to
> >> identify what code in t/helper is unit testing C code and what code is
> >> really about helping other tests in the t/t*.sh scripts. It would be
> >> nice if you could give an example of each kind of code.
> >
> > In my opinion, helper/test-ctype.c is a unit test file containing a set of unit tests for character classification functions,
>
> Right.
>
> > while helper/test-dir-iterator.c is a unit test file which works together with the t/t0066-dir-iterator.sh file to iterate through a directory and give details on its contents. It likely is used for testing and inspecting directory structures and file types within a specified path.
>
> Actually "t/helper/test-dir-iterator.c" and "t/t0066-dir-iterator.sh"
> are used together to test the code in "dir-iterator.h" and
> "dir-iterator.c", so it's kind of special. Ideally this could be
> ported to the unit test framework as the goal is to test quite low
> level code (instead of a full git command for example), but I am not
> sure how easy it would be, and if it would even be worth it.
>
> >> An example of how you would migrate parts of a test, or how a migrated
> >> test would look like, would be nice.
> >
> > I'd first create a new test file, then include "test-libtap/tap.h" and "test-tool.h" header files, then I would initialize TAP with this command plan_tests(x), where x defines the number of tests to be run inside that file.
> > Below the plan_tests();, I'd migrate and edit my specific test function and requirements, and after that, I'd add my "done_testing();" and then "return exit_status();"
>
> These are quite good guidelines, but not quite an example.
>
Using the test-ctype.c file as an example, the migrated unit test file
would look like this:
#include "test-tool.h"
#include "test-libtap/tap.h"
#include <stdio.h>
#include <string.h>
static int is_in(const char *s, int ch) {
if (ch == '\0')
return ch == *s;
if (*s == '\0')
s++;
return !!strchr(s, ch);
}
int main() {
// Init TAP with the number of planned tests (1 test per character class)
plan(12);
// Character classes
char *classes[] = {
"DIGIT", " \n\r\t",
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ",
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789",
"*?[\\", "$()*+.?[\\^{|", "!\"#%&',-/:;<=>@_`~",
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f"
"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f"
"\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f"
"\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f"
"\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f"
"\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f"
"\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f"
"\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f",
};
// Test each character class
for (int i = 0; i < 12; i++) {
char description[50];
sprintf(description, "Testing %s character class", classes[i]);
for (int ch = 0; ch < 256; ch++) {
int result = is_in(classes[i], ch);
if (ch == EOF)
ok(result == is_in(classes[i], ch), description);
else
is_int(result, is_in(classes[i], ch), description);
}
}
// After testing all
done_testing();
return exit_status();
}
> Thanks,
> Christian.
^ permalink raw reply
* Re: [RFC][Outreachy] Seeking Git Community Feedback on My Application
From: Christian Couder @ 2023-10-28 12:37 UTC (permalink / raw)
To: Isoken Ibizugbe; +Cc: git
In-Reply-To: <CAJHH8bGK28Fc+VG3uxgC5sGgFEAw6_6AEtusgmw7c4Vz0iGF_g@mail.gmail.com>
On Sat, Oct 28, 2023 at 12:41 PM Isoken Ibizugbe <isokenjune@gmail.com> wrote:
>
> On Sat, Oct 28, 2023 at 9:07 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > > #define DIGIT "0123456789"
> > >
> > > static void t_digit_type(void)
> > > {
> > > int i;
> > > const char *digits = DIGIT;
> > > for (i = 0; digits[i]; i++)
> > > {
> > > check_int(isdigit(digits[i]), ==, 0);
> > > }
> >
> > This tests that isdigit() returns 0 for each of the characters in
> > "0123456789", but first I think isdigit() should return 1, not 0 for
> > those characters.
>
> yes, that is true. should I send a re-roll?
Yes, please.
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2023, #03; Fri, 6)
From: René Scharfe @ 2023-10-28 12:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqcyxn3gas.fsf@gitster.g>
Am 09.10.23 um 18:35 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> Am 07.10.23 um 10:20 schrieb Junio C Hamano:
>>> * rs/parse-options-value-int (2023-09-18) 2 commits
>>> - parse-options: use and require int pointer for OPT_CMDMODE
>>> - parse-options: add int value pointer to struct option
>>> ...
>> I don't mind removing this topic from seen for now; it's not ready, yet.
>
> After seeing the discussion moved to giving a more type safe enum
> support and then somehow convesation seemed to have petered out, I
> was wondering if we figured out the problem space enough to see an
> updated version with well defined scope and good problem
> description.
It's a simple idea with a broad scope: Replace void pointers or fortify
them by tracking type information across their use in order to get
compile time type checks for the entire code base. Give developers the
basic guardrails they already have everywhere else also in generic code.
Adding such type checks is easy, but fixing all the places using
incompatible pointers is a lot of churn. Option parsing, sorting and
many other callback functions would have to be fortified. The
OPT_CMDMODE patches were a test balloon for a small subset of the void
space. Extending them to cover more or even all of the int options may
give a better picture of the price for such a feature.
Will it uncover user-visible bugs? So far it hasn't, not directly.
Perhaps ASan suffices and we don't need to care about exotic platforms
that would punish our type sins.
Dealing with the OPT_CMDMODE code prompted to me to come up with ways
to improve its error messages, though. These patches took me
surprisingly long to prepare (plus my Git time is quite limited these
days). I'll send them separately.
> I do not mind keeping it on 'seen', especially if
> these two early steps are not expected to change. It is not like
> they are causing any maintenance burden.
The OPT_CMDMODE error message improvements change the code a lot and
conflict with these parse-options patches. Please drop the latter and
let's discuss the former first, as their user-visible changes are more
tangible and useful. I'll send an adjusted and perhaps bigger type
safety test balloon eventually.
René
^ permalink raw reply
* [PATCH] reflog: fix expire --single-worktree
From: René Scharfe @ 2023-10-28 11:58 UTC (permalink / raw)
To: Git List; +Cc: John Cai
33d7bdd645 (builtin/reflog.c: use parse-options api for expire, delete
subcommands, 2022-01-06) broke the option --single-worktree of git
reflog expire and added a non-printable short flag for it, presumably by
accident. While before it set the variable "all_worktrees" to 0, now it
sets it to 1, its default value. --no-single-worktree is required now
to set it to 0.
Fix it by replacing the variable with one that has the opposite meaning,
to avoid the negation and its potential for confusion. The new variable
"single_worktree" directly captures whether --single-worktree was given.
Also remove the unprintable short flag SOH (start of heading) because it
is undocumented, hard to use and is likely to have been added by mistake
in connection with the negation bug above.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
builtin/reflog.c | 6 +++---
t/t1410-reflog.sh | 23 +++++++++++++++++++++++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index df63a5892e..21337292f5 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -243,7 +243,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
{
struct cmd_reflog_expire_cb cmd = { 0 };
timestamp_t now = time(NULL);
- int i, status, do_all, all_worktrees = 1;
+ int i, status, do_all, single_worktree = 0;
unsigned int flags = 0;
int verbose = 0;
reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
@@ -268,7 +268,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "stale-fix", &cmd.stalefix,
N_("prune any reflog entries that point to broken commits")),
OPT_BOOL(0, "all", &do_all, N_("process the reflogs of all references")),
- OPT_BOOL(1, "single-worktree", &all_worktrees,
+ OPT_BOOL(0, "single-worktree", &single_worktree,
N_("limits processing to reflogs from the current worktree only")),
OPT_END()
};
@@ -318,7 +318,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
worktrees = get_worktrees();
for (p = worktrees; *p; p++) {
- if (!all_worktrees && !(*p)->is_current)
+ if (single_worktree && !(*p)->is_current)
continue;
collected.worktree = *p;
refs_for_each_reflog(get_worktree_ref_store(*p),
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 6c45965b1e..09e7f3cdac 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -446,6 +446,29 @@ test_expect_success 'expire with multiple worktrees' '
)
'
+test_expect_success 'expire one of multiple worktrees' '
+ git init main-wt2 &&
+ (
+ cd main-wt2 &&
+ test_tick &&
+ test_commit foo &&
+ git worktree add link-wt &&
+ test_tick &&
+ test_commit -C link-wt foobar &&
+ test_tick &&
+ test-tool ref-store worktree:link-wt for-each-reflog-ent HEAD \
+ >expect-link-wt &&
+ git reflog expire --verbose --all --expire=$test_tick \
+ --single-worktree &&
+ test-tool ref-store worktree:main for-each-reflog-ent HEAD \
+ >actual-main &&
+ test-tool ref-store worktree:link-wt for-each-reflog-ent HEAD \
+ >actual-link-wt &&
+ test_must_be_empty actual-main &&
+ test_cmp expect-link-wt actual-link-wt
+ )
+'
+
test_expect_success REFFILES 'empty reflog' '
test_when_finished "rm -rf empty" &&
git init empty &&
--
2.42.0
^ permalink raw reply related
* [PATCH] am, rebase: fix arghelp syntax of --empty
From: René Scharfe @ 2023-10-28 11:58 UTC (permalink / raw)
To: Git List; +Cc: Elijah Newren, 徐沛文 (Aleen)
Use parentheses and pipes to present alternatives in the argument help
for the --empty options of git am and git rebase, like in the rest of
the documentation.
While at it remove a stray use of the enum empty_action value
STOP_ON_EMPTY_COMMIT to indicate that no short option is present.
While it has a value of 0 and thus there is no user-visible change,
that enum is not meant to hold short option characters. Hard-code 0,
like we do for other options without a short option.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
Documentation/git-rebase.txt | 4 ++--
builtin/am.c | 2 +-
builtin/rebase.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e7b39ad244..b4526ca246 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -289,7 +289,7 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
---empty={drop,keep,ask}::
+--empty=(drop|keep|ask)::
How to handle commits that are not empty to start and are not
clean cherry-picks of any upstream commit, but which become
empty after rebasing (because they contain a subset of already
@@ -695,7 +695,7 @@ be dropped automatically with `--no-keep-empty`).
Similar to the apply backend, by default the merge backend drops
commits that become empty unless `-i`/`--interactive` is specified (in
which case it stops and asks the user what to do). The merge backend
-also has an `--empty={drop,keep,ask}` option for changing the behavior
+also has an `--empty=(drop|keep|ask)` option for changing the behavior
of handling commits that become empty.
Directory rename detection
diff --git a/builtin/am.c b/builtin/am.c
index 6655059a57..9bb73d1671 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2420,7 +2420,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
{ OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"),
N_("GPG-sign commits"),
PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
- OPT_CALLBACK_F(STOP_ON_EMPTY_COMMIT, "empty", &state.empty_type, "{stop,drop,keep}",
+ OPT_CALLBACK_F(0, "empty", &state.empty_type, "(stop|drop|keep)",
N_("how to handle empty patches"),
PARSE_OPT_NONEG, am_option_parse_empty),
OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing,
diff --git a/builtin/rebase.c b/builtin/rebase.c
index f990811614..ad7baedd4a 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1147,7 +1147,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
"instead of ignoring them"),
1, PARSE_OPT_HIDDEN),
OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
- OPT_CALLBACK_F(0, "empty", &options, "{drop,keep,ask}",
+ OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|ask)",
N_("how to handle commits that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
OPT_CALLBACK_F('k', "keep-empty", &options, NULL,
--
2.42.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox