* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:29 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Oswald Buddenhagen, git
In-Reply-To: <ZTbJiIkIyXwWK8JP.jacob@initialcommit.io>
On 2023-10-23 21:29, Jacob Stopak wrote:
> However, now that I'm thinking about it maybe it could somehow be
> included
> in the Hints feature? I honestly don't know exactly when the hints are
> currently invoked or how much detail they go into, but what just popped
> into my head is kindof a "universal dry run" option, which would show
> the
> user the --table format hint when they invoke an applicable command,
> and
> prompt them if they actually want to run it.
That's a good idea, having the tables and dry runs mentioned in a
well-placed hint that could, of course, be disabled through git
configuration.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 20:47 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, Jacob Stopak, git
In-Reply-To: <18c7b1bea07d5f3878f4466b8e133da1@manjaro.org>
>On 2023-10-23 21:01, Junio C Hamano wrote:
>> One thing that nobody seems to have raised that disturbs me is that
>> even though there may be educational value in having such a
>> "feature", having to carry the extra code to implement in Git incurs
>> extra cost.
>
that's the case for literally every feature. you're just noticing it
here, because from your expert perspective it seems redundant with
pre-existing functionality. so assuming that you actually care about
newbie UX, you are at least not convinced that it would be a significant
help.
i'm not totally convinced either, which is why i wrote of "valid
experiment" upthread. but i don't think that measuring the actual impact
is all that interesting unless the feature turns out to be actually
excessively costly in the long run.
On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>That's exactly why I already wrote that the original author of the table
>patches, if those would become accepted, should commit in advance to
>maintaining that as the new git feature.
>
that's seems just a wee bit unfair (the bar isn't put that high for
other features), and it's not realistically enforcable anyway.
regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:51 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Jacob Stopak, git
In-Reply-To: <ZTbVY7Nf+DTYqHky@ugly>
On 2023-10-23 22:19, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 12:29:12PM -0700, Jacob Stopak wrote:
>> Those arrows showing how things move only really apply to "simulating"
>> (dry runs) for specific commands like add, restore, rm, commit, stash,
>> etc, so making the --table proposal a default status output would
>> still
>> miss those scenarios.
>>
> you're too focused on the status quo of your own tool. :-)
> there is really nothing that would speak against the real commands
> reporting what they just *actually did*. this would seem rather
> helpful for noobs and other insecure users.
> if one really wanted, "you can also use this with --dry-run" could be
> part of the hint that would say how to turn off the extra verbosity
> (or just the hint itself, if one likes the verbosity).
The hint should be about how to turn the tables and verbosity on, not
how to get rid of it.
> one could even go one step further and put at least the destructive
> commands into interactive/confirmation mode by default. but that's
> probably a bridge too far, as it would be potentially habit-forming in
> a bad way.
>
> regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 20:59 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Junio C Hamano, Jacob Stopak, git
In-Reply-To: <ZTbb7bHkFOOyBT6+@ugly>
> On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> That's exactly why I already wrote that the original author of
>> the table patches, if those would become accepted, should commit
>> in advance to maintaining that as the new git feature.
>>
> that's seems just a wee bit unfair (the bar isn't put that high for
> other features), and it's not realistically enforcable anyway.
Well, it's a bit disruptive new feature, which is also quite extensive
and can rather easily become obsolete if not maintained in the long run.
Perhaps we should hear Jacob's thoughts about that as well.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-23 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dragan Simic, Oswald Buddenhagen, git
In-Reply-To: <xmqqfs21noxx.fsf@gitster.g>
On Mon, Oct 23, 2023 at 12:01:14PM -0700, Junio C Hamano wrote:
> Ah, OK, now I see where your "--table" is coming from ;-).
> "git-sim" was exactly what I thought about when I saw it, and I did
> not know that "--table" came from the same set of brain cells.
Haha the set is not quite the same I'm sure I've lost many over the
course of this year. But the survivors are doing their best.
> One thing that nobody seems to have raised that disturbs me is that
> even though there may be educational value in having such a
> "feature", having to carry the extra code to implement in Git incurs
> extra cost. I was reasonably happy when I saw that "git-sim" was
> done as a totally separate entity exactly for this reason.
Erm, not to get too sappy here but I'd love to maintain anything related
to this that gets implemented, in whatever form that turns out to be.
I already spend way too much of my free time working on Git-related
things so making this contribution to Git itself would mean a lot to me.
Starting Git-Sim as a separate entity made sense to me because:
* I had no idea whether anyone wanted something like this, so it
would have made for a pretty weak argument to the community. (It
turned out way more users were interested than I thought).
* It's written in Python and relies on a dependency library called
Manim, so I thought it wouldn't make any sense to try and wedge
that into the Git codebase.
* The output is presentation quality images / video animations, which
is unlike anything I've seen outputted by any Git command. (I didn't
explore what it would take to do something similar in C).
The main downsides to Git-Sim as a separate tool are:
* The main downside is lack of reach. Not being Git-native means only
a tiny fraction of Git users will ever know it exists.
* There is technically no guarantee that a simulated output actually
corresponds to what the command will do, as highlighted by this
comment on Hacker News: "Next HN post - "I destroyed my repo - but
it WORKED in Git-Sim!"
* As Git changes over time, Git-Sim is destined to be a step behind.
* Certain commands (like the networked commands fetch/push/pull/etc)
are not easy to simulate without doing horrible things like cloning
a new copy of the repo behind the scenes, running the desired
operation on it, and checking the result. I assume things like this
would be a lot easier to do within Git.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Oswald Buddenhagen @ 2023-10-23 21:14 UTC (permalink / raw)
To: Dragan Simic; +Cc: Jacob Stopak, git
In-Reply-To: <d55eaf41f55bdff0b5ae734e5d7e6724@manjaro.org>
On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>The hint should be about how to turn the tables and verbosity on, not
>how to get rid of it.
>
but that's just backwards. a noob shouldn't be bothered with putting the
tool into noob-friendly mode, neither actually nor "just"
psychologically. switching to "expert" mode should be the conscious
effort. and making it opt-in wouldn't even save the experts any
annoyance, as they need to opt out from the hint anyway.
regards
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 21:19 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: Jacob Stopak, git
In-Reply-To: <ZTbiU+SwvGi9972S@ugly>
On 2023-10-23 23:14, Oswald Buddenhagen wrote:
> On Mon, Oct 23, 2023 at 10:51:31PM +0200, Dragan Simic wrote:
>> The hint should be about how to turn the tables and verbosity on, not
>> how to get rid of it.
>
> but that's just backwards. a noob shouldn't be bothered with putting
> the tool into noob-friendly mode, neither actually nor "just"
> psychologically. switching to "expert" mode should be the conscious
> effort. and making it opt-in wouldn't even save the experts any
> annoyance, as they need to opt out from the hint anyway.
Let's focus on non-noobs as well, which could go nuts by having some
strange tables displayed after updating git. For the beginners, having
something configured is already inevitable, e.g. their credentials, so
they could also turn on the tables if they wanted so.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Jacob Stopak @ 2023-10-23 21:23 UTC (permalink / raw)
To: Dragan Simic; +Cc: Oswald Buddenhagen, Junio C Hamano, git
In-Reply-To: <c8e437eacdb883f612ae44e43c95602f@manjaro.org>
On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
> > > That's exactly why I already wrote that the original author of
> > > the table patches, if those would become accepted, should commit
> > > in advance to maintaining that as the new git feature.
> > >
> > that's seems just a wee bit unfair (the bar isn't put that high for
> > other features), and it's not realistically enforcable anyway.
>
> Well, it's a bit disruptive new feature, which is also quite extensive and
> can rather easily become obsolete if not maintained in the long run.
> Perhaps we should hear Jacob's thoughts about that as well.
I would love to work on implementing this and maintaining it! I spend
too much time doing Git things for no reason anyway so why not? :D
But... I could die at the hands of an angry user and then you would have
to take over, Dragan...
^ permalink raw reply
* Re: [RFC PATCH 0/5] Introduce -t, --table for status/add commands
From: Dragan Simic @ 2023-10-23 21:26 UTC (permalink / raw)
To: Jacob Stopak; +Cc: Oswald Buddenhagen, Junio C Hamano, git
In-Reply-To: <ZTbkUUbmp56fgc+a.jacob@initialcommit.io>
On 2023-10-23 23:23, Jacob Stopak wrote:
> On Mon, Oct 23, 2023 at 10:59:08PM +0200, Dragan Simic wrote:
>> > On Mon, Oct 23, 2023 at 09:04:49PM +0200, Dragan Simic wrote:
>> > > That's exactly why I already wrote that the original author of
>> > > the table patches, if those would become accepted, should commit
>> > > in advance to maintaining that as the new git feature.
>> > >
>> > that's seems just a wee bit unfair (the bar isn't put that high for
>> > other features), and it's not realistically enforcable anyway.
>>
>> Well, it's a bit disruptive new feature, which is also quite extensive
>> and
>> can rather easily become obsolete if not maintained in the long run.
>> Perhaps we should hear Jacob's thoughts about that as well.
>
> I would love to work on implementing this and maintaining it! I spend
> too much time doing Git things for no reason anyway so why not? :D
Awesome! Quite frankly, I expected to hear that. :)
> But... I could die at the hands of an angry user and then you would
> have
> to take over, Dragan...
Or I could instead try to help you fending off angry git users? :)
^ permalink raw reply
* [PATCH v3 0/7] log: decorate pseudorefs and other refs
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231022214432.56325-1-andy.koppe@gmail.com>
This patch series implements decoration with pseudorefs and adds three
slots to the color.decorate.<slot> config:
- 'symbol' for coloring the punctuation symbols used around the refs in
decorations, which currently use the same color as the commit hash.
- 'ref' for coloring refs other than branches, remote-tracking branches,
tags and the stash, which currently are not colored when included in
decorations through custom decoration filter options.
- 'pseudoref' for coloring pseudorefs such as ORIG_HEAD or MERGE_HEAD.
CI: https://github.com/ak2/git/actions/runs/6618230811
Andy Koppe (7):
config: restructure color.decorate documentation
log: use designated inits for decoration_colors
log: add color.decorate.symbol config variable
log: add color.decorate.ref config variable
refs: add pseudorefs array and iteration functions
refs: exempt pseudorefs from pattern prefixing
log: add color.decorate.pseudoref config variable
Documentation/config/color.txt | 32 +++++++-
commit.h | 3 +
log-tree.c | 57 ++++++++++----
refs.c | 59 +++++++++++++--
refs.h | 5 ++
..._--decorate=full_--clear-decorations_--all | 4 +-
...f.log_--decorate_--clear-decorations_--all | 4 +-
t/t4202-log.sh | 21 +++---
t/t4207-log-decoration-colors.sh | 74 +++++++++++--------
9 files changed, 192 insertions(+), 67 deletions(-)
--
2.42.GIT
^ permalink raw reply
* [PATCH v3 1/7] config: restructure color.decorate documentation
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
List color.decorate slots in git-config documentation one-by-one in the
same way as color.grep slots, to aid readability and make it easier to
add slots.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 1795b2d16b..3453703f9b 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -74,10 +74,25 @@ color.diff.<slot>::
`oldBold`, and `newBold` (see linkgit:git-range-diff[1] for details).
color.decorate.<slot>::
- Use customized color for 'git log --decorate' output. `<slot>` is one
- of `branch`, `remoteBranch`, `tag`, `stash` or `HEAD` for local
- branches, remote-tracking branches, tags, stash and HEAD, respectively
- and `grafted` for grafted commits.
+ Use customized color for the output of 'git log --decorate' as well as
+ the `%d`, `%D` and `%(decorate)` placeholders in custom log formats,
+ whereby `<slot>` specifies which decoration elements the color applies
+ to:
++
+--
+`HEAD`;;
+ the current HEAD
+`branch`;;
+ local branches
+`remoteBranch`;;
+ remote-tracking branches
+`tag`;;
+ lightweight and annotated tags
+`stash`;;
+ the stash ref
+`grafted`;;
+ grafted and replaced commits
+--
color.grep::
When set to `always`, always highlight matches. When `false` (or
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 2/7] log: use designated inits for decoration_colors
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Use designated initializers instead of comments to denote the slots in
the decoration_colors array for holding color settings, to make it
consistent with the immediately following color_decorate_slots array
and reduce the likelihood of mistakes when extending them.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
log-tree.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 504da6b519..8bdf889f02 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -34,13 +34,13 @@ static int decoration_loaded;
static int decoration_flags;
static char decoration_colors[][COLOR_MAXLEN] = {
- GIT_COLOR_RESET,
- GIT_COLOR_BOLD_GREEN, /* REF_LOCAL */
- GIT_COLOR_BOLD_RED, /* REF_REMOTE */
- GIT_COLOR_BOLD_YELLOW, /* REF_TAG */
- GIT_COLOR_BOLD_MAGENTA, /* REF_STASH */
- GIT_COLOR_BOLD_CYAN, /* REF_HEAD */
- GIT_COLOR_BOLD_BLUE, /* GRAFTED */
+ [DECORATION_NONE] = GIT_COLOR_RESET,
+ [DECORATION_REF_LOCAL] = GIT_COLOR_BOLD_GREEN,
+ [DECORATION_REF_REMOTE] = GIT_COLOR_BOLD_RED,
+ [DECORATION_REF_TAG] = GIT_COLOR_BOLD_YELLOW,
+ [DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
+ [DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
+ [DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
};
static const char *color_decorate_slots[] = {
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 4/7] log: add color.decorate.ref config variable
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Refs other than branches, remote-tracking branches, tags and the stash
do not appear in log decorations by default, but they can be shown by
using decoration filter options such as --clear-decorations or
log.initialDecorationSet. However, they would appear without color.
Add config variable color.decorate.ref for such refs, defaulting to bold
magenta, which is the same as refs/stash.
Document the new variable on the git-config page and amend
t4207-log-decoration-colors.sh to test it.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 5 +++++
commit.h | 1 +
log-tree.c | 4 +++-
t/t4207-log-decoration-colors.sh | 9 +++++++--
4 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index cc0a881125..005a2bdb03 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -90,11 +90,16 @@ color.decorate.<slot>::
lightweight and annotated tags
`stash`;;
the stash ref
+`ref`;;
+ any other refs (not shown by default)
`grafted`;;
grafted and replaced commits
`symbol`;;
punctuation symbols surrounding the other elements
--
++
+(Variable `log.initialDecorationSet` or linkgit:git-log[1] option
+`--clear-decorations` can be used to show all refs.)
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/commit.h b/commit.h
index cb13e4d5ba..f6b2125fc4 100644
--- a/commit.h
+++ b/commit.h
@@ -54,6 +54,7 @@ enum decoration_type {
DECORATION_REF_REMOTE,
DECORATION_REF_TAG,
DECORATION_REF_STASH,
+ DECORATION_REF,
DECORATION_REF_HEAD,
DECORATION_GRAFTED,
DECORATION_SYMBOL,
diff --git a/log-tree.c b/log-tree.c
index 890024f205..fb3d87b83d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -39,6 +39,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
[DECORATION_REF_REMOTE] = GIT_COLOR_BOLD_RED,
[DECORATION_REF_TAG] = GIT_COLOR_BOLD_YELLOW,
[DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
+ [DECORATION_REF] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
[DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
[DECORATION_SYMBOL] = GIT_COLOR_NIL,
@@ -49,6 +50,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_REMOTE] = "remoteBranch",
[DECORATION_REF_TAG] = "tag",
[DECORATION_REF_STASH] = "stash",
+ [DECORATION_REF] = "ref",
[DECORATION_REF_HEAD] = "HEAD",
[DECORATION_GRAFTED] = "grafted",
[DECORATION_SYMBOL] = "symbol",
@@ -151,7 +153,7 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
int i;
struct object *obj;
enum object_type objtype;
- enum decoration_type deco_type = DECORATION_NONE;
+ enum decoration_type deco_type = DECORATION_REF;
struct decoration_filter *filter = (struct decoration_filter *)cb_data;
const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f4173b6114..4b51e34f8b 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -17,6 +17,7 @@ test_expect_success setup '
git config color.decorate.remoteBranch red &&
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
+ git config color.decorate.ref blue &&
git config color.decorate.grafted black &&
git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
@@ -28,11 +29,13 @@ test_expect_success setup '
c_remoteBranch="<RED>" &&
c_tag="<BOLD;REVERSE;YELLOW>" &&
c_stash="<MAGENTA>" &&
+ c_ref="<BLUE>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
c_symbol="<WHITE>" &&
test_commit A &&
+ git update-ref refs/foo A &&
git clone . other &&
(
cd other &&
@@ -65,10 +68,12 @@ ${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
+${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
EOF
- git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
+ git log --first-parent --no-abbrev --decorate --clear-decorations \
+ --oneline --color=always --all >actual &&
cmp_filtered_decorations
'
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 5/7] refs: add pseudorefs array and iteration functions
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Define const array 'pseudorefs' with the names of the pseudorefs that
are documented in gitrevisions.1, and add functions for_each_pseudoref()
and refs_for_each_pseudoref() for iterating over them.
The functions process the pseudorefs in the same way as head_ref() and
refs_head_ref() process HEAD, invoking an each_ref_fn callback on each
pseudoref that exists.
This is in preparation for adding pseudorefs to log decorations.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
refs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
refs.h | 5 +++++
2 files changed, 47 insertions(+)
diff --git a/refs.c b/refs.c
index fcae5dddc6..aa7e4c02c5 100644
--- a/refs.c
+++ b/refs.c
@@ -65,6 +65,21 @@ static unsigned char refname_disposition[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
};
+/*
+ * List of documented pseudorefs. This needs to be kept in sync with the list
+ * in Documentation/revisions.txt.
+ */
+static const char *const pseudorefs[] = {
+ "FETCH_HEAD",
+ "ORIG_HEAD",
+ "MERGE_HEAD",
+ "REBASE_HEAD",
+ "CHERRY_PICK_HEAD",
+ "REVERT_HEAD",
+ "BISECT_HEAD",
+ "AUTO_MERGE",
+};
+
struct ref_namespace_info ref_namespace[] = {
[NAMESPACE_HEAD] = {
.ref = "HEAD",
@@ -1549,6 +1564,33 @@ int head_ref(each_ref_fn fn, void *cb_data)
return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
}
+int refs_for_each_pseudoref(struct ref_store *refs,
+ each_ref_fn fn, void *cb_data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pseudorefs); i++) {
+ struct object_id oid;
+ int flag;
+
+ if (refs_resolve_ref_unsafe(refs, pseudorefs[i],
+ RESOLVE_REF_READING, &oid, &flag)) {
+ int ret = fn(pseudorefs[i], &oid, flag, cb_data);
+
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+int for_each_pseudoref(each_ref_fn fn, void *cb_data)
+{
+ return refs_for_each_pseudoref(get_main_ref_store(the_repository),
+ fn, cb_data);
+}
+
struct ref_iterator *refs_ref_iterator_begin(
struct ref_store *refs,
const char *prefix,
diff --git a/refs.h b/refs.h
index 23211a5ea1..7b55cced31 100644
--- a/refs.h
+++ b/refs.h
@@ -320,6 +320,8 @@ typedef int each_repo_ref_fn(struct repository *r,
*/
int refs_head_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
+int refs_for_each_pseudoref(struct ref_store *refs,
+ each_ref_fn fn, void *cb_data);
int refs_for_each_ref(struct ref_store *refs,
each_ref_fn fn, void *cb_data);
int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
@@ -334,6 +336,9 @@ int refs_for_each_remote_ref(struct ref_store *refs,
/* just iterates the head ref. */
int head_ref(each_ref_fn fn, void *cb_data);
+/* iterates pseudorefs. */
+int for_each_pseudoref(each_ref_fn fn, void *cb_data);
+
/* iterates all refs. */
int for_each_ref(each_ref_fn fn, void *cb_data);
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 3/7] log: add color.decorate.symbol config variable
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Add new color.decorate.symbol config variable for determining the
color of the prefix, suffix, separator and pointer symbols used in
log --decorate output and related log format placeholders, to allow
them to be colored differently from commit hashes.
For backward compatibility, fall back to the commit hash color that can
be specified with the color.diff.commit variable if the new variable is
not provided.
Add the variable to the color.decorate.<slot> documentation.
Amend t4207-log-decoration-colors.sh to test it. Put ${c_reset} elements
in the expected output at the end of lines for consistency.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 2 ++
commit.h | 1 +
log-tree.c | 15 ++++++---
t/t4207-log-decoration-colors.sh | 58 +++++++++++++++++---------------
4 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 3453703f9b..cc0a881125 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,8 @@ color.decorate.<slot>::
the stash ref
`grafted`;;
grafted and replaced commits
+`symbol`;;
+ punctuation symbols surrounding the other elements
--
color.grep::
diff --git a/commit.h b/commit.h
index 28928833c5..cb13e4d5ba 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
DECORATION_REF_STASH,
DECORATION_REF_HEAD,
DECORATION_GRAFTED,
+ DECORATION_SYMBOL,
};
void add_name_decoration(enum decoration_type type, const char *name, struct object *obj);
diff --git a/log-tree.c b/log-tree.c
index 8bdf889f02..890024f205 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
[DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
[DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
+ [DECORATION_SYMBOL] = GIT_COLOR_NIL,
};
static const char *color_decorate_slots[] = {
@@ -50,6 +51,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_STASH] = "stash",
[DECORATION_REF_HEAD] = "HEAD",
[DECORATION_GRAFTED] = "grafted",
+ [DECORATION_SYMBOL] = "symbol",
};
static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
@@ -312,7 +314,7 @@ void format_decorations(struct strbuf *sb,
{
const struct name_decoration *decoration;
const struct name_decoration *current_and_HEAD;
- const char *color_commit, *color_reset;
+ const char *color_symbol, *color_reset;
const char *prefix = " (";
const char *suffix = ")";
@@ -337,7 +339,10 @@ void format_decorations(struct strbuf *sb,
tag = opts->tag;
}
- color_commit = diff_get_color(use_color, DIFF_COMMIT);
+ color_symbol = decorate_get_color(use_color, DECORATION_SYMBOL);
+ if (color_is_nil(color_symbol))
+ color_symbol = diff_get_color(use_color, DIFF_COMMIT);
+
color_reset = decorate_get_color(use_color, DECORATION_NONE);
current_and_HEAD = current_pointed_by_HEAD(decoration);
@@ -352,7 +357,7 @@ void format_decorations(struct strbuf *sb,
decorate_get_color(use_color, decoration->type);
if (*prefix) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, prefix);
strbuf_addstr(sb, color_reset);
}
@@ -369,7 +374,7 @@ void format_decorations(struct strbuf *sb,
if (current_and_HEAD &&
decoration->type == DECORATION_REF_HEAD) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, pointer);
strbuf_addstr(sb, color_reset);
strbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));
@@ -382,7 +387,7 @@ void format_decorations(struct strbuf *sb,
decoration = decoration->next;
}
if (*suffix) {
- strbuf_addstr(sb, color_commit);
+ strbuf_addstr(sb, color_symbol);
strbuf_addstr(sb, suffix);
strbuf_addstr(sb, color_reset);
}
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 21986a866d..f4173b6114 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
git config color.decorate.grafted black &&
+ git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
c_reset="<RESET>" &&
@@ -29,6 +30,7 @@ test_expect_success setup '
c_stash="<MAGENTA>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
+ c_symbol="<WHITE>" &&
test_commit A &&
git clone . other &&
@@ -53,17 +55,17 @@ cmp_filtered_decorations () {
# to this test since it does not contain any decoration, hence --first-parent
test_expect_success 'commit decorations colored correctly' '
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_commit}, \
-${c_reset}${c_remoteBranch}other/main${c_reset}${c_commit})${c_reset} A1
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_stash}refs/stash${c_reset}${c_commit})${c_reset} On main: Changes to A.t
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
+${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always --all >actual &&
@@ -78,14 +80,14 @@ test_expect_success 'test coloring with replace-objects' '
git replace HEAD~1 HEAD~2 &&
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit})${c_reset} D
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} B
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol})${c_reset} D
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}C${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} B
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
@@ -104,15 +106,15 @@ test_expect_success 'test coloring with grafted commit' '
git replace --graft HEAD HEAD~2 &&
cat >expect <<-EOF &&
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}\
-${c_commit} -> ${c_reset}${c_branch}main${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_commit}, \
-${c_reset}${c_grafted}replaced${c_reset}${c_commit})${c_reset} D
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_commit}, \
-${c_reset}${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_commit})${c_reset} B
- ${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}\
-${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}${c_HEAD}HEAD${c_reset}\
+${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}D${c_reset}${c_symbol}, ${c_reset}\
+${c_grafted}replaced${c_reset}${c_symbol})${c_reset} D
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
+ ${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol})${c_reset} A
EOF
git log --first-parent --no-abbrev --decorate --oneline --color=always HEAD >actual &&
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 6/7] refs: exempt pseudorefs from pattern prefixing
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
In normalize_glob_ref(), don't prefix pseudorefs with "refs/", thereby
implementing a NEEDSWORK from b877e617e6e5.
This is in preparation for showing pseudorefs in log decorations, as
they are not matched as intended in decoration filters otherwise. The
function is only used in load_ref_decorations().
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
refs.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index aa7e4c02c5..fbd15a8cff 100644
--- a/refs.c
+++ b/refs.c
@@ -565,13 +565,16 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix,
if (prefix)
strbuf_addstr(&normalized_pattern, prefix);
- else if (!starts_with(pattern, "refs/") &&
- strcmp(pattern, "HEAD"))
- strbuf_addstr(&normalized_pattern, "refs/");
- /*
- * NEEDSWORK: Special case other symrefs such as REBASE_HEAD,
- * MERGE_HEAD, etc.
- */
+ else if (!starts_with(pattern, "refs/") && strcmp(pattern, "HEAD")) {
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pseudorefs); i++)
+ if (!strcmp(pattern, pseudorefs[i]))
+ break;
+
+ if (i == ARRAY_SIZE(pseudorefs))
+ strbuf_addstr(&normalized_pattern, "refs/");
+ }
strbuf_addstr(&normalized_pattern, pattern);
strbuf_strip_suffix(&normalized_pattern, "/");
--
2.42.GIT
^ permalink raw reply related
* [PATCH v3 7/7] log: add color.decorate.pseudoref config variable
From: Andy Koppe @ 2023-10-23 22:11 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, Andy Koppe
In-Reply-To: <20231023221143.72489-1-andy.koppe@gmail.com>
Add the ability to show pseudorefs such as ORIG_HEAD and MERGE_HEAD in
log decorations. Add config variable color.decorate.pseudoref to
determine their color, defaulting to bold cyan, which is the same as
HEAD.
They will not be shown unless the default decoration filtering is
overridden with relevant log options such as --clear-decorations or
log.initialDecorationSet.
Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Documentation/config/color.txt | 4 +++-
commit.h | 1 +
log-tree.c | 24 +++++++++++++++++++
..._--decorate=full_--clear-decorations_--all | 4 ++--
...f.log_--decorate_--clear-decorations_--all | 4 ++--
t/t4202-log.sh | 21 +++++++++-------
t/t4207-log-decoration-colors.sh | 13 +++++++---
7 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/Documentation/config/color.txt b/Documentation/config/color.txt
index 005a2bdb03..7af7d65f76 100644
--- a/Documentation/config/color.txt
+++ b/Documentation/config/color.txt
@@ -92,6 +92,8 @@ color.decorate.<slot>::
the stash ref
`ref`;;
any other refs (not shown by default)
+`pseudoref`;;
+ pseudorefs such as ORIG_HEAD or MERGE_HEAD (not shown by default)
`grafted`;;
grafted and replaced commits
`symbol`;;
@@ -99,7 +101,7 @@ color.decorate.<slot>::
--
+
(Variable `log.initialDecorationSet` or linkgit:git-log[1] option
-`--clear-decorations` can be used to show all refs.)
+`--clear-decorations` can be used to show all refs and pseudorefs.)
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/commit.h b/commit.h
index f6b2125fc4..44dd3ce19b 100644
--- a/commit.h
+++ b/commit.h
@@ -56,6 +56,7 @@ enum decoration_type {
DECORATION_REF_STASH,
DECORATION_REF,
DECORATION_REF_HEAD,
+ DECORATION_REF_PSEUDO,
DECORATION_GRAFTED,
DECORATION_SYMBOL,
};
diff --git a/log-tree.c b/log-tree.c
index fb3d87b83d..65ebb74d40 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -41,6 +41,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
[DECORATION_REF_STASH] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF] = GIT_COLOR_BOLD_MAGENTA,
[DECORATION_REF_HEAD] = GIT_COLOR_BOLD_CYAN,
+ [DECORATION_REF_PSEUDO] = GIT_COLOR_BOLD_CYAN,
[DECORATION_GRAFTED] = GIT_COLOR_BOLD_BLUE,
[DECORATION_SYMBOL] = GIT_COLOR_NIL,
};
@@ -52,6 +53,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_REF_STASH] = "stash",
[DECORATION_REF] = "ref",
[DECORATION_REF_HEAD] = "HEAD",
+ [DECORATION_REF_PSEUDO] = "pseudoref",
[DECORATION_GRAFTED] = "grafted",
[DECORATION_SYMBOL] = "symbol",
};
@@ -208,6 +210,27 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
return 0;
}
+static int add_pseudoref_decoration(const char *refname,
+ const struct object_id *oid,
+ int flags UNUSED,
+ void *cb_data)
+{
+ struct object *obj;
+ enum object_type objtype;
+ struct decoration_filter *filter = (struct decoration_filter *)cb_data;
+
+ if (filter && !ref_filter_match(refname, filter))
+ return 0;
+
+ objtype = oid_object_info(the_repository, oid, NULL);
+ if (objtype < 0)
+ return 0;
+
+ obj = lookup_object_by_type(the_repository, oid, objtype);
+ add_name_decoration(DECORATION_REF_PSEUDO, refname, obj);
+ return 0;
+}
+
static int add_graft_decoration(const struct commit_graft *graft,
void *cb_data UNUSED)
{
@@ -236,6 +259,7 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)
decoration_loaded = 1;
decoration_flags = flags;
for_each_ref(add_ref_decoration, filter);
+ for_each_pseudoref(add_pseudoref_decoration, filter);
head_ref(add_ref_decoration, filter);
for_each_commit_graft(add_graft_decoration, filter);
}
diff --git a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
index 1c030a6554..7d16978e7f 100644
--- a/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate=full_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date: Mon Jun 26 00:04:00 2006 +0000
Merge branch 'side'
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, refs/heads/side)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:03:00 2006 +0000
Side
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:02:00 2006 +0000
diff --git a/t/t4013/diff.log_--decorate_--clear-decorations_--all b/t/t4013/diff.log_--decorate_--clear-decorations_--all
index 88be82cce3..4f9be50ce0 100644
--- a/t/t4013/diff.log_--decorate_--clear-decorations_--all
+++ b/t/t4013/diff.log_--decorate_--clear-decorations_--all
@@ -33,13 +33,13 @@ Date: Mon Jun 26 00:04:00 2006 +0000
Merge branch 'side'
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (FETCH_HEAD, side)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:03:00 2006 +0000
Side
-commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0 (ORIG_HEAD)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:02:00 2006 +0000
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd2..b14da62e3e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -927,7 +927,7 @@ test_expect_success 'multiple decorate-refs' '
test_expect_success 'decorate-refs-exclude with glob' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b)
octopus-a (tag: octopus-a)
@@ -944,7 +944,7 @@ test_expect_success 'decorate-refs-exclude with glob' '
test_expect_success 'decorate-refs-exclude without globs' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b, octopus-b)
octopus-a (tag: octopus-a, octopus-a)
@@ -961,7 +961,7 @@ test_expect_success 'decorate-refs-exclude without globs' '
test_expect_success 'multiple decorate-refs-exclude' '
cat >expect.decorate <<-\EOF &&
Merge-tag-reach (HEAD -> main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: seventh)
octopus-b (tag: octopus-b)
octopus-a (tag: octopus-a)
@@ -1022,10 +1022,12 @@ test_expect_success 'decorate-refs-exclude and simplify-by-decoration' '
EOF
git log -n6 --decorate=short --pretty="tformat:%f%d" \
--decorate-refs-exclude="*octopus*" \
+ --decorate-refs-exclude="ORIG_HEAD" \
--simplify-by-decoration >actual &&
test_cmp expect.decorate actual &&
- git -c log.excludeDecoration="*octopus*" log \
- -n6 --decorate=short --pretty="tformat:%f%d" \
+ git -c log.excludeDecoration="*octopus*" \
+ -c log.excludeDecoration="ORIG_HEAD" \
+ log -n6 --decorate=short --pretty="tformat:%f%d" \
--simplify-by-decoration >actual &&
test_cmp expect.decorate actual
'
@@ -1067,9 +1069,10 @@ test_expect_success 'decorate-refs and simplify-by-decoration without output' '
test_cmp expect actual
'
-test_expect_success 'decorate-refs-exclude HEAD' '
+test_expect_success 'decorate-refs-exclude HEAD ORIG_HEAD' '
git log --decorate=full --oneline \
- --decorate-refs-exclude="HEAD" >actual &&
+ --decorate-refs-exclude="HEAD" \
+ --decorate-refs-exclude="ORIG_HEAD" >actual &&
! grep HEAD actual
'
@@ -1107,7 +1110,7 @@ test_expect_success '--clear-decorations overrides defaults' '
cat >expect.all <<-\EOF &&
Merge-tag-reach (HEAD -> refs/heads/main)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
seventh (tag: refs/tags/seventh)
octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
@@ -1139,7 +1142,7 @@ test_expect_success '--clear-decorations clears previous exclusions' '
cat >expect.all <<-\EOF &&
Merge-tag-reach (HEAD -> refs/heads/main)
reach (tag: refs/tags/reach, refs/heads/reach)
- Merge-tags-octopus-a-and-octopus-b
+ Merge-tags-octopus-a-and-octopus-b (ORIG_HEAD)
octopus-b (tag: refs/tags/octopus-b, refs/heads/octopus-b)
octopus-a (tag: refs/tags/octopus-a, refs/heads/octopus-a)
seventh (tag: refs/tags/seventh)
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 4b51e34f8b..0b32e0bb8e 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
git config color.decorate.tag "reverse bold yellow" &&
git config color.decorate.stash magenta &&
git config color.decorate.ref blue &&
+ git config color.decorate.pseudoref "bold cyan" &&
git config color.decorate.grafted black &&
git config color.decorate.symbol white &&
git config color.decorate.HEAD cyan &&
@@ -30,6 +31,7 @@ test_expect_success setup '
c_tag="<BOLD;REVERSE;YELLOW>" &&
c_stash="<MAGENTA>" &&
c_ref="<BLUE>" &&
+ c_pseudoref="<BOLD;CYAN>" &&
c_HEAD="<CYAN>" &&
c_grafted="<BLACK>" &&
c_symbol="<WHITE>" &&
@@ -46,7 +48,10 @@ test_expect_success setup '
test_commit B &&
git tag v1.0 &&
echo >>A.t &&
- git stash save Changes to A.t
+ git stash save Changes to A.t &&
+ git reset other/main &&
+ git reset ORIG_HEAD &&
+ git revert --no-commit @~
'
cmp_filtered_decorations () {
@@ -63,17 +68,19 @@ ${c_symbol} -> ${c_reset}${c_branch}main${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}v1.0${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}B${c_reset}${c_symbol})${c_reset} B
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}ORIG_HEAD${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}A1${c_reset}${c_symbol}, ${c_reset}\
${c_remoteBranch}other/main${c_reset}${c_symbol})${c_reset} A1
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
${c_stash}refs/stash${c_reset}${c_symbol})${c_reset} On main: Changes to A.t
${c_commit}COMMIT_ID${c_reset}${c_symbol} (${c_reset}\
+${c_pseudoref}REVERT_HEAD${c_reset}${c_symbol}, ${c_reset}\
${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_symbol}, ${c_reset}\
${c_ref}refs/foo${c_reset}${c_symbol})${c_reset} A
EOF
- git log --first-parent --no-abbrev --decorate --clear-decorations \
- --oneline --color=always --all >actual &&
+ git log --first-parent --no-abbrev --decorate --color=always \
+ --decorate-refs-exclude=FETCH_HEAD --oneline --all >actual &&
cmp_filtered_decorations
'
--
2.42.GIT
^ permalink raw reply related
* Re: [PATCH 0/7] log: decorate pseudorefs and other refs
From: Andy Koppe @ 2023-10-23 22:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, stolee
In-Reply-To: <xmqqpm16p4t3.fsf@gitster.g>
On 23/10/2023 01:20, Junio C Hamano wrote:
> Andy Koppe <andy.koppe@gmail.com> writes:
>
>>> [2/7] is a trivial readability improvement. It obviously should be
>>> left outside the scope of this series, but we should notice
>>> the same pattern in similar color tables (e.g., wt-status.c
>>> has one, diff.c has another) and perform the same clean-up as
>>> a #leftoverbits item.
>>
>> Okay, I've removed that commit in v2. (I should have mentioned in the
>> commit message that it was triggered by the inconsistency with the
>> immediately following color_decorate_slots array, which uses
>> designated initializers.)
>
> Sorry, that is not what I meant. [2/7] as a preliminary clean-up to
> work in the same area does make very much sense. What I meant to be
> "outside the scope" was to make similar fixes to other color tables
> that this series does not care about.
Ah, sorry for misreading. Commit reinstated in v3.
>> Fair enough, although the array already contains HEAD and refs/stash
>> as singletons.
>
> But these deserve to be singletons, don't they? There is no other
> thing that behaves like HEAD; there is no other thing that behaves
> like stash; and they do not behave like each other.
They do indeed, but arguably the pseudorefs are singletons rather than a
namespace like refs/heads as well, as there is a defined and documented
set of them.
>> I've rewritten things to not touch the ref_namespace array.
>
> Well, the namespace_info mechanism still may be a good place to have
> the necessary information; it may be that the current implementation
> detail of how a given ref is classified to one of the namespaces is
> too limiting---it essentially allows the string match with the .ref
> member. But we can imagine that it could be extended a bit, e.g.
>
> struct ref_namespace_info {
> char *ref;
> int (*membership)(const char *, const struct ref_namespace_info *);
> ... other members ...;
> };
>
> where the .membership member is used in add_ref_decoration() to
> determine the membership of a given "refname" to the namespace "i"
> perhaps like so:
>
> struct ref_namespace_info *info = &ref_namespace[i];
>
> if (!info->decoration)
> continue;
> + if (info->membership) {
> + if (info->membership(refname, info)) {
> + deco_type = info->decoration;
> + break;
> + }
> + } else if (info->exact) {
> - if (info->exact) {
> if (!strcmp(refname, info->ref)) {
> deco_type = info_decoration;
> break;
> }
>
> Then you can arrange the pseudoref class to use .membership function
> perhaps like this:
>
> static int pseudoref_namespace_membership(
> const char *refname, const struct ref_namespace_info *info UNUSED
> )
> {
> return is_pseudoref(refname);
> }
>
> and make them all into a single class.
That's an interesting idea, but I'm not convinced it would buy us much,
while also potentially complicating things for any other uses of the
ref_namespace array.
My premise here is that we do need a list of the documented pseudorefs,
so that we can iterate through them and add the ones that do exist to
the decorations, whereby I admit that shoe-horning that list into the
ref_namespace array wasn't a good idea. If that premise is wrong, and
there's a better way to discover the pseudorefs, the following might be
moot.
Sending each found pseudoref through add_ref_decoration() and its lookup
of ref_namespace would just confirm what we already know: it's a
pseudoref. Which is why both my initial attempt and the current one
don't actually invoke add_ref_decoration() for them.
Could you have a closer look at the current design? It handles the
pseudorefs separately from proper refs, with their own iteration and
callback functions, which I think makes for simpler more self-contained
changes than v1 or the approach suggested above.
> Having said that, I do not think it makes much sense to decorate a
> commit off of refs/stash, as the true richeness of the stash is not
> in its history but in its reflog, which the decoration code does not
> dig into. But obviously it is not a part of the topic we are
> discussing (unless, of course, we are not "adding" new decoration
> sources and colors, but we are improving the decoration sources and
> colors by adding new useful ones while retiring existing useless
> ones).
I agree refs/stash is a weird one, and that it could be subsumed into
the color.decoration.ref setting for 'refs/*' that I'm adding here,
which is also why I chose the same default color for it. I'd be happy to
drop color.decoration.stash if the minor break in compatibility for
anyone who has customized it is acceptable. The setting would be quietly
ignored.
Another related thought: the '--clear-decorations' option of git-log
seems unfortunately named as it suggests the opposite of what it
actually does, which is to enable all decorations (unless subsequently
constrained with '--decorate-refs{,--exclude}=...').
Regards,
Andy
^ permalink raw reply
* [PATCH v5 0/5] merge-ort: implement support for packing objects together
From: Taylor Blau @ 2023-10-23 22:44 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1697736516.git.me@ttaylorr.com>
(Rebased onto the current tip of 'master', which is ceadf0f3cf (The
twentieth batch, 2023-10-20)).
This series implements support for a new merge-tree option,
`--write-pack`, which causes any newly-written objects to be packed
together instead of being stored individually as loose.
This is a minor follow-up that could be taken instead of v4 (though the
changes between these two most recent rounds are stylistic and a matter
of subjective opinion).
This moves us the bulk_checkin_source structure introduced in response
to Junio's suggestion during the last round further in the OOP
direction. Instead of switching on the enum type of the source, have
function pointers for read() and seek() respectively.
The functionality at the end is the same, but this avoids some of the
namespacing issues that Peff pointed out while looking at v4. But I
think that this approach ended up being less heavy-weight than I had
originally imagined, so I think that this version is a worthwhile
improvement over v4.
Beyond that, the changes since last time can be viewed in the range-diff
below. Thanks in advance for any review!
[1]: https://lore.kernel.org/git/xmqq34y7plj4.fsf@gitster.g/
Taylor Blau (5):
bulk-checkin: extract abstract `bulk_checkin_source`
bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
builtin/merge-tree.c: implement support for `--write-pack`
Documentation/git-merge-tree.txt | 4 +
builtin/merge-tree.c | 5 +
bulk-checkin.c | 197 +++++++++++++++++++++++++++----
bulk-checkin.h | 8 ++
merge-ort.c | 42 +++++--
merge-recursive.h | 1 +
t/t4301-merge-tree-write-tree.sh | 93 +++++++++++++++
7 files changed, 316 insertions(+), 34 deletions(-)
Range-diff against v4:
1: 97bb6e9f59 ! 1: 696aa027e4 bulk-checkin: extract abstract `bulk_checkin_source`
@@ bulk-checkin.c: static int already_written(struct bulk_checkin_packfile *state,
}
+struct bulk_checkin_source {
-+ enum { SOURCE_FILE } type;
++ off_t (*read)(struct bulk_checkin_source *, void *, size_t);
++ off_t (*seek)(struct bulk_checkin_source *, off_t);
+
-+ /* SOURCE_FILE fields */
-+ int fd;
++ union {
++ struct {
++ int fd;
++ } from_fd;
++ } data;
+
-+ /* common fields */
+ size_t size;
+ const char *path;
+};
+
-+static off_t bulk_checkin_source_seek_to(struct bulk_checkin_source *source,
-+ off_t offset)
++static off_t bulk_checkin_source_read_from_fd(struct bulk_checkin_source *source,
++ void *buf, size_t nr)
+{
-+ switch (source->type) {
-+ case SOURCE_FILE:
-+ return lseek(source->fd, offset, SEEK_SET);
-+ default:
-+ BUG("unknown bulk-checkin source: %d", source->type);
-+ }
++ return read_in_full(source->data.from_fd.fd, buf, nr);
+}
+
-+static ssize_t bulk_checkin_source_read(struct bulk_checkin_source *source,
-+ void *buf, size_t nr)
++static off_t bulk_checkin_source_seek_from_fd(struct bulk_checkin_source *source,
++ off_t offset)
+{
-+ switch (source->type) {
-+ case SOURCE_FILE:
-+ return read_in_full(source->fd, buf, nr);
-+ default:
-+ BUG("unknown bulk-checkin source: %d", source->type);
-+ }
++ return lseek(source->data.from_fd.fd, offset, SEEK_SET);
++}
++
++static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
++ int fd, size_t size,
++ const char *path)
++{
++ memset(source, 0, sizeof(struct bulk_checkin_source));
++
++ source->read = bulk_checkin_source_read_from_fd;
++ source->seek = bulk_checkin_source_seek_from_fd;
++
++ source->data.from_fd.fd = fd;
++
++ source->size = size;
++ source->path = path;
+}
+
/*
@@ bulk-checkin.c: static int stream_blob_to_pack(struct bulk_checkin_packfile *sta
- ssize_t read_result = read_in_full(fd, ibuf, rsize);
+ ssize_t read_result;
+
-+ read_result = bulk_checkin_source_read(source, ibuf,
-+ rsize);
++ read_result = source->read(source, ibuf, rsize);
if (read_result < 0)
- die_errno("failed to read from '%s'", path);
+ die_errno("failed to read from '%s'",
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
unsigned header_len;
struct hashfile_checkpoint checkpoint = {0};
struct pack_idx_entry *idx = NULL;
-+ struct bulk_checkin_source source = {
-+ .type = SOURCE_FILE,
-+ .fd = fd,
-+ .size = size,
-+ .path = path,
-+ };
++ struct bulk_checkin_source source;
++
++ init_bulk_checkin_source_from_fd(&source, fd, size, path);
seekback = lseek(fd, 0, SEEK_CUR);
if (seekback == (off_t) -1)
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
state->offset = checkpoint.offset;
flush_bulk_checkin_packfile(state);
- if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
-+ if (bulk_checkin_source_seek_to(&source, seekback) == (off_t)-1)
++ if (source.seek(&source, seekback) == (off_t)-1)
return error("cannot seek back");
}
the_hash_algo->final_oid_fn(result_oid, &ctx);
2: 9d633df339 < -: ---------- bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
3: d5bbd7810e ! 2: 596bd028a7 bulk-checkin: refactor deflate routine to accept a `bulk_checkin_source`
@@ Metadata
Author: Taylor Blau <me@ttaylorr.com>
## Commit message ##
- bulk-checkin: refactor deflate routine to accept a `bulk_checkin_source`
+ bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
- Prepare for a future change where we will want to use a routine very
- similar to the existing `deflate_blob_to_pack()` but over arbitrary
- sources (i.e. either open file-descriptors, or a location in memory).
+ The existing `stream_blob_to_pack()` function is named based on the fact
+ that it knows only how to stream blobs into a bulk-checkin pack.
- Extract out a common "deflate_obj_to_pack()" routine that acts on a
- bulk_checkin_source, instead of a (int, size_t) pair. Then rewrite
- `deflate_blob_to_pack()` in terms of it.
+ But there is no longer anything in this function which prevents us from
+ writing objects of arbitrary types to the bulk-checkin pack. Prepare to
+ write OBJ_TREEs by removing this assumption, adding an `enum
+ object_type` parameter to this function's argument list, and renaming it
+ to `stream_obj_to_pack()` accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bulk-checkin.c ##
+@@ bulk-checkin.c: static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
+ * status before calling us just in case we ask it to call us again
+ * with a new pack.
+ */
+-static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
+- git_hash_ctx *ctx, off_t *already_hashed_to,
+- struct bulk_checkin_source *source,
+- unsigned flags)
++static int stream_obj_to_pack(struct bulk_checkin_packfile *state,
++ git_hash_ctx *ctx, off_t *already_hashed_to,
++ struct bulk_checkin_source *source,
++ enum object_type type, unsigned flags)
+ {
+ git_zstream s;
+ unsigned char ibuf[16384];
+@@ bulk-checkin.c: static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
+
+ git_deflate_init(&s, pack_compression_level);
+
+- hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB,
+- size);
++ hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), type, size);
+ s.next_out = obuf + hdrlen;
+ s.avail_out = sizeof(obuf) - hdrlen;
+
@@ bulk-checkin.c: static void prepare_to_stream(struct bulk_checkin_packfile *state,
die_errno("unable to write pack header");
}
@@ bulk-checkin.c: static void prepare_to_stream(struct bulk_checkin_packfile *stat
unsigned header_len;
struct hashfile_checkpoint checkpoint = {0};
struct pack_idx_entry *idx = NULL;
-- struct bulk_checkin_source source = {
-- .type = SOURCE_FILE,
-- .fd = fd,
-- .size = size,
-- .path = path,
-- };
+- struct bulk_checkin_source source;
+- init_bulk_checkin_source_from_fd(&source, fd, size, path);
+-
- seekback = lseek(fd, 0, SEEK_CUR);
- if (seekback == (off_t) -1)
- return error("cannot find the current offset");
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
prepare_to_stream(state, flags);
if (idx) {
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
+ idx->offset = state->offset;
crc32_begin(state->f);
}
- if (!stream_obj_to_pack(state, &ctx, &already_hashed_to,
-- &source, OBJ_BLOB, flags))
+- if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
+- &source, flags))
++ if (!stream_obj_to_pack(state, &ctx, &already_hashed_to,
+ source, type, flags))
break;
/*
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
hashfile_truncate(state->f, &checkpoint);
state->offset = checkpoint.offset;
flush_bulk_checkin_packfile(state);
-- if (bulk_checkin_source_seek_to(&source, seekback) == (off_t)-1)
-+ if (bulk_checkin_source_seek_to(source, seekback) == (off_t)-1)
+- if (source.seek(&source, seekback) == (off_t)-1)
++ if (source->seek(source, seekback) == (off_t)-1)
return error("cannot seek back");
}
the_hash_algo->final_oid_fn(result_oid, &ctx);
@@ bulk-checkin.c: static int deflate_blob_to_pack(struct bulk_checkin_packfile *st
+ int fd, size_t size,
+ const char *path, unsigned flags)
+{
-+ struct bulk_checkin_source source = {
-+ .type = SOURCE_FILE,
-+ .fd = fd,
-+ .size = size,
-+ .path = path,
-+ };
-+ off_t seekback = lseek(fd, 0, SEEK_CUR);
++ struct bulk_checkin_source source;
++ off_t seekback;
++
++ init_bulk_checkin_source_from_fd(&source, fd, size, path);
++
++ seekback = lseek(fd, 0, SEEK_CUR);
+ if (seekback == (off_t) -1)
+ return error("cannot find the current offset");
+
4: e427fe6ad3 < -: ---------- bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
5: 48095afe80 ! 3: d8cf8e4395 bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
@@ Metadata
## Commit message ##
bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
- Now that we have factored out many of the common routines necessary to
- index a new object into a pack created by the bulk-checkin machinery, we
- can introduce a variant of `index_blob_bulk_checkin()` that acts on
- blobs whose contents we can fit in memory.
+ Introduce `index_blob_bulk_checkin_incore()` which allows streaming
+ arbitrary blob contents from memory into the bulk-checkin pack.
+
+ In order to support streaming from a location in memory, we must
+ implement a new kind of bulk_checkin_source that does just that. These
+ implementation in spread out across:
+
+ - init_bulk_checkin_source_incore()
+ - bulk_checkin_source_read_incore()
+ - bulk_checkin_source_seek_incore()
+
+ Note that, unlike file descriptors, which manage their own offset
+ internally, we have to keep track of how many bytes we've read out of
+ the buffer, and make sure we don't read past the end of the buffer.
This will be useful in a couple of more commits in order to provide the
`merge-tree` builtin with a mechanism to create a new pack containing
@@ Commit message
Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bulk-checkin.c ##
+@@ bulk-checkin.c: struct bulk_checkin_source {
+ struct {
+ int fd;
+ } from_fd;
++ struct {
++ const void *buf;
++ size_t nr_read;
++ } incore;
+ } data;
+
+ size_t size;
+@@ bulk-checkin.c: static off_t bulk_checkin_source_seek_from_fd(struct bulk_checkin_source *source
+ return lseek(source->data.from_fd.fd, offset, SEEK_SET);
+ }
+
++static off_t bulk_checkin_source_read_incore(struct bulk_checkin_source *source,
++ void *buf, size_t nr)
++{
++ const unsigned char *src = source->data.incore.buf;
++
++ if (source->data.incore.nr_read > source->size)
++ BUG("read beyond bulk-checkin source buffer end "
++ "(%"PRIuMAX" > %"PRIuMAX")",
++ (uintmax_t)source->data.incore.nr_read,
++ (uintmax_t)source->size);
++
++ if (nr > source->size - source->data.incore.nr_read)
++ nr = source->size - source->data.incore.nr_read;
++
++ src += source->data.incore.nr_read;
++
++ memcpy(buf, src, nr);
++ source->data.incore.nr_read += nr;
++ return nr;
++}
++
++static off_t bulk_checkin_source_seek_incore(struct bulk_checkin_source *source,
++ off_t offset)
++{
++ if (!(0 <= offset && offset < source->size))
++ return (off_t)-1;
++ source->data.incore.nr_read = offset;
++ return source->data.incore.nr_read;
++}
++
+ static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
+ int fd, size_t size,
+ const char *path)
+@@ bulk-checkin.c: static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
+ source->path = path;
+ }
+
++static void init_bulk_checkin_source_incore(struct bulk_checkin_source *source,
++ const void *buf, size_t size,
++ const char *path)
++{
++ memset(source, 0, sizeof(struct bulk_checkin_source));
++
++ source->read = bulk_checkin_source_read_incore;
++ source->seek = bulk_checkin_source_seek_incore;
++
++ source->data.incore.buf = buf;
++ source->data.incore.nr_read = 0;
++
++ source->size = size;
++ source->path = path;
++}
++
+ /*
+ * Read the contents from 'source' for 'size' bytes, streaming it to the
+ * packfile in state while updating the hash in ctx. Signal a failure
@@ bulk-checkin.c: static int deflate_obj_to_pack(struct bulk_checkin_packfile *state,
return 0;
}
@@ bulk-checkin.c: static int deflate_obj_to_pack(struct bulk_checkin_packfile *sta
+ const char *path, enum object_type type,
+ unsigned flags)
+{
-+ struct bulk_checkin_source source = {
-+ .type = SOURCE_INCORE,
-+ .buf = buf,
-+ .size = size,
-+ .read = 0,
-+ .path = path,
-+ };
++ struct bulk_checkin_source source;
++
++ init_bulk_checkin_source_incore(&source, buf, size, path);
+
+ return deflate_obj_to_pack(state, result_oid, &source, type, 0, flags);
+}
6: 60568f9281 = 4: 2670192802 bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
7: b9be9df122 ! 5: 3595db76a5 builtin/merge-tree.c: implement support for `--write-pack`
@@ Documentation/git-merge-tree.txt: OPTIONS
## builtin/merge-tree.c ##
@@
- #include "quote.h"
#include "tree.h"
#include "config.h"
+ #include "strvec.h"
+#include "bulk-checkin.h"
static int line_termination = '\n';
@@ builtin/merge-tree.c: struct merge_tree_options {
- int show_messages;
int name_only;
int use_stdin;
+ struct merge_options merge_options;
+ int write_pack;
};
static int real_merge(struct merge_tree_options *o,
@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
- init_merge_options(&opt, the_repository);
+ _("not something we can merge"));
opt.show_rename_progress = 0;
+ opt.write_pack = o->write_pack;
@@ builtin/merge-tree.c: static int real_merge(struct merge_tree_options *o,
opt.branch1 = branch1;
opt.branch2 = branch2;
@@ builtin/merge-tree.c: int cmd_merge_tree(int argc, const char **argv, const char *prefix)
- &merge_base,
- N_("commit"),
N_("specify a merge-base for the merge")),
+ OPT_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
+ N_("option for selected merge strategy")),
+ OPT_BOOL(0, "write-pack", &o.write_pack,
+ N_("write new objects to a pack instead of as loose")),
OPT_END()
base-commit: ceadf0f3cf51550166a387ec8508bb55e7883057
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply
* [PATCH v5 1/5] bulk-checkin: extract abstract `bulk_checkin_source`
From: Taylor Blau @ 2023-10-23 22:44 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1698101088.git.me@ttaylorr.com>
A future commit will want to implement a very similar routine as in
`stream_blob_to_pack()` with two notable changes:
- Instead of streaming just OBJ_BLOBs, this new function may want to
stream objects of arbitrary type.
- Instead of streaming the object's contents from an open
file-descriptor, this new function may want to "stream" its contents
from memory.
To avoid duplicating a significant chunk of code between the existing
`stream_blob_to_pack()`, extract an abstract `bulk_checkin_source`. This
concept currently is a thin layer of `lseek()` and `read_in_full()`, but
will grow to understand how to perform analogous operations when writing
out an object's contents from memory.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 65 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 57 insertions(+), 8 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 6ce62999e5..174a6c24e4 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -140,8 +140,49 @@ static int already_written(struct bulk_checkin_packfile *state, struct object_id
return 0;
}
+struct bulk_checkin_source {
+ off_t (*read)(struct bulk_checkin_source *, void *, size_t);
+ off_t (*seek)(struct bulk_checkin_source *, off_t);
+
+ union {
+ struct {
+ int fd;
+ } from_fd;
+ } data;
+
+ size_t size;
+ const char *path;
+};
+
+static off_t bulk_checkin_source_read_from_fd(struct bulk_checkin_source *source,
+ void *buf, size_t nr)
+{
+ return read_in_full(source->data.from_fd.fd, buf, nr);
+}
+
+static off_t bulk_checkin_source_seek_from_fd(struct bulk_checkin_source *source,
+ off_t offset)
+{
+ return lseek(source->data.from_fd.fd, offset, SEEK_SET);
+}
+
+static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
+ int fd, size_t size,
+ const char *path)
+{
+ memset(source, 0, sizeof(struct bulk_checkin_source));
+
+ source->read = bulk_checkin_source_read_from_fd;
+ source->seek = bulk_checkin_source_seek_from_fd;
+
+ source->data.from_fd.fd = fd;
+
+ source->size = size;
+ source->path = path;
+}
+
/*
- * Read the contents from fd for size bytes, streaming it to the
+ * Read the contents from 'source' for 'size' bytes, streaming it to the
* packfile in state while updating the hash in ctx. Signal a failure
* by returning a negative value when the resulting pack would exceed
* the pack size limit and this is not the first object in the pack,
@@ -157,7 +198,7 @@ static int already_written(struct bulk_checkin_packfile *state, struct object_id
*/
static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
git_hash_ctx *ctx, off_t *already_hashed_to,
- int fd, size_t size, const char *path,
+ struct bulk_checkin_source *source,
unsigned flags)
{
git_zstream s;
@@ -167,22 +208,27 @@ static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
int status = Z_OK;
int write_object = (flags & HASH_WRITE_OBJECT);
off_t offset = 0;
+ size_t size = source->size;
git_deflate_init(&s, pack_compression_level);
- hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, size);
+ hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB,
+ size);
s.next_out = obuf + hdrlen;
s.avail_out = sizeof(obuf) - hdrlen;
while (status != Z_STREAM_END) {
if (size && !s.avail_in) {
ssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf);
- ssize_t read_result = read_in_full(fd, ibuf, rsize);
+ ssize_t read_result;
+
+ read_result = source->read(source, ibuf, rsize);
if (read_result < 0)
- die_errno("failed to read from '%s'", path);
+ die_errno("failed to read from '%s'",
+ source->path);
if (read_result != rsize)
die("failed to read %d bytes from '%s'",
- (int)rsize, path);
+ (int)rsize, source->path);
offset += rsize;
if (*already_hashed_to < offset) {
size_t hsize = offset - *already_hashed_to;
@@ -258,6 +304,9 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
unsigned header_len;
struct hashfile_checkpoint checkpoint = {0};
struct pack_idx_entry *idx = NULL;
+ struct bulk_checkin_source source;
+
+ init_bulk_checkin_source_from_fd(&source, fd, size, path);
seekback = lseek(fd, 0, SEEK_CUR);
if (seekback == (off_t) -1)
@@ -283,7 +332,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
crc32_begin(state->f);
}
if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
- fd, size, path, flags))
+ &source, flags))
break;
/*
* Writing this object to the current pack will make
@@ -295,7 +344,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
hashfile_truncate(state->f, &checkpoint);
state->offset = checkpoint.offset;
flush_bulk_checkin_packfile(state);
- if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
+ if (source.seek(&source, seekback) == (off_t)-1)
return error("cannot seek back");
}
the_hash_algo->final_oid_fn(result_oid, &ctx);
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply related
* [PATCH v5 2/5] bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
From: Taylor Blau @ 2023-10-23 22:44 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1698101088.git.me@ttaylorr.com>
The existing `stream_blob_to_pack()` function is named based on the fact
that it knows only how to stream blobs into a bulk-checkin pack.
But there is no longer anything in this function which prevents us from
writing objects of arbitrary types to the bulk-checkin pack. Prepare to
write OBJ_TREEs by removing this assumption, adding an `enum
object_type` parameter to this function's argument list, and renaming it
to `stream_obj_to_pack()` accordingly.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 61 +++++++++++++++++++++++++++++---------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 174a6c24e4..79776e679e 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -196,10 +196,10 @@ static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
* status before calling us just in case we ask it to call us again
* with a new pack.
*/
-static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
- git_hash_ctx *ctx, off_t *already_hashed_to,
- struct bulk_checkin_source *source,
- unsigned flags)
+static int stream_obj_to_pack(struct bulk_checkin_packfile *state,
+ git_hash_ctx *ctx, off_t *already_hashed_to,
+ struct bulk_checkin_source *source,
+ enum object_type type, unsigned flags)
{
git_zstream s;
unsigned char ibuf[16384];
@@ -212,8 +212,7 @@ static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
git_deflate_init(&s, pack_compression_level);
- hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB,
- size);
+ hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), type, size);
s.next_out = obuf + hdrlen;
s.avail_out = sizeof(obuf) - hdrlen;
@@ -293,27 +292,23 @@ static void prepare_to_stream(struct bulk_checkin_packfile *state,
die_errno("unable to write pack header");
}
-static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
- struct object_id *result_oid,
- int fd, size_t size,
- const char *path, unsigned flags)
+
+static int deflate_obj_to_pack(struct bulk_checkin_packfile *state,
+ struct object_id *result_oid,
+ struct bulk_checkin_source *source,
+ enum object_type type,
+ off_t seekback,
+ unsigned flags)
{
- off_t seekback, already_hashed_to;
+ off_t already_hashed_to = 0;
git_hash_ctx ctx;
unsigned char obuf[16384];
unsigned header_len;
struct hashfile_checkpoint checkpoint = {0};
struct pack_idx_entry *idx = NULL;
- struct bulk_checkin_source source;
- init_bulk_checkin_source_from_fd(&source, fd, size, path);
-
- seekback = lseek(fd, 0, SEEK_CUR);
- if (seekback == (off_t) -1)
- return error("cannot find the current offset");
-
- header_len = format_object_header((char *)obuf, sizeof(obuf),
- OBJ_BLOB, size);
+ header_len = format_object_header((char *)obuf, sizeof(obuf), type,
+ source->size);
the_hash_algo->init_fn(&ctx);
the_hash_algo->update_fn(&ctx, obuf, header_len);
the_hash_algo->init_fn(&checkpoint.ctx);
@@ -322,8 +317,6 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
if ((flags & HASH_WRITE_OBJECT) != 0)
CALLOC_ARRAY(idx, 1);
- already_hashed_to = 0;
-
while (1) {
prepare_to_stream(state, flags);
if (idx) {
@@ -331,8 +324,8 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
idx->offset = state->offset;
crc32_begin(state->f);
}
- if (!stream_blob_to_pack(state, &ctx, &already_hashed_to,
- &source, flags))
+ if (!stream_obj_to_pack(state, &ctx, &already_hashed_to,
+ source, type, flags))
break;
/*
* Writing this object to the current pack will make
@@ -344,7 +337,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
hashfile_truncate(state->f, &checkpoint);
state->offset = checkpoint.offset;
flush_bulk_checkin_packfile(state);
- if (source.seek(&source, seekback) == (off_t)-1)
+ if (source->seek(source, seekback) == (off_t)-1)
return error("cannot seek back");
}
the_hash_algo->final_oid_fn(result_oid, &ctx);
@@ -366,6 +359,24 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
return 0;
}
+static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
+ struct object_id *result_oid,
+ int fd, size_t size,
+ const char *path, unsigned flags)
+{
+ struct bulk_checkin_source source;
+ off_t seekback;
+
+ init_bulk_checkin_source_from_fd(&source, fd, size, path);
+
+ seekback = lseek(fd, 0, SEEK_CUR);
+ if (seekback == (off_t) -1)
+ return error("cannot find the current offset");
+
+ return deflate_obj_to_pack(state, result_oid, &source, OBJ_BLOB,
+ seekback, flags);
+}
+
void prepare_loose_object_bulk_checkin(void)
{
/*
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply related
* [PATCH v5 3/5] bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
From: Taylor Blau @ 2023-10-23 22:45 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1698101088.git.me@ttaylorr.com>
Introduce `index_blob_bulk_checkin_incore()` which allows streaming
arbitrary blob contents from memory into the bulk-checkin pack.
In order to support streaming from a location in memory, we must
implement a new kind of bulk_checkin_source that does just that. These
implementation in spread out across:
- init_bulk_checkin_source_incore()
- bulk_checkin_source_read_incore()
- bulk_checkin_source_seek_incore()
Note that, unlike file descriptors, which manage their own offset
internally, we have to keep track of how many bytes we've read out of
the buffer, and make sure we don't read past the end of the buffer.
This will be useful in a couple of more commits in order to provide the
`merge-tree` builtin with a mechanism to create a new pack containing
any objects it created during the merge, instead of storing those
objects individually as loose.
Similar to the existing `index_blob_bulk_checkin()` function, the
entrypoint delegates to `deflate_obj_to_pack_incore()`. That function in
turn delegates to deflate_obj_to_pack(), which is responsible for
formatting the pack header and then deflating the contents into the
pack.
Consistent with the rest of the bulk-checkin mechanism, there are no
direct tests here. In future commits when we expose this new
functionality via the `merge-tree` builtin, we will test it indirectly
there.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
bulk-checkin.h | 4 +++
2 files changed, 79 insertions(+)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 79776e679e..b728210bc7 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -148,6 +148,10 @@ struct bulk_checkin_source {
struct {
int fd;
} from_fd;
+ struct {
+ const void *buf;
+ size_t nr_read;
+ } incore;
} data;
size_t size;
@@ -166,6 +170,36 @@ static off_t bulk_checkin_source_seek_from_fd(struct bulk_checkin_source *source
return lseek(source->data.from_fd.fd, offset, SEEK_SET);
}
+static off_t bulk_checkin_source_read_incore(struct bulk_checkin_source *source,
+ void *buf, size_t nr)
+{
+ const unsigned char *src = source->data.incore.buf;
+
+ if (source->data.incore.nr_read > source->size)
+ BUG("read beyond bulk-checkin source buffer end "
+ "(%"PRIuMAX" > %"PRIuMAX")",
+ (uintmax_t)source->data.incore.nr_read,
+ (uintmax_t)source->size);
+
+ if (nr > source->size - source->data.incore.nr_read)
+ nr = source->size - source->data.incore.nr_read;
+
+ src += source->data.incore.nr_read;
+
+ memcpy(buf, src, nr);
+ source->data.incore.nr_read += nr;
+ return nr;
+}
+
+static off_t bulk_checkin_source_seek_incore(struct bulk_checkin_source *source,
+ off_t offset)
+{
+ if (!(0 <= offset && offset < source->size))
+ return (off_t)-1;
+ source->data.incore.nr_read = offset;
+ return source->data.incore.nr_read;
+}
+
static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
int fd, size_t size,
const char *path)
@@ -181,6 +215,22 @@ static void init_bulk_checkin_source_from_fd(struct bulk_checkin_source *source,
source->path = path;
}
+static void init_bulk_checkin_source_incore(struct bulk_checkin_source *source,
+ const void *buf, size_t size,
+ const char *path)
+{
+ memset(source, 0, sizeof(struct bulk_checkin_source));
+
+ source->read = bulk_checkin_source_read_incore;
+ source->seek = bulk_checkin_source_seek_incore;
+
+ source->data.incore.buf = buf;
+ source->data.incore.nr_read = 0;
+
+ source->size = size;
+ source->path = path;
+}
+
/*
* Read the contents from 'source' for 'size' bytes, streaming it to the
* packfile in state while updating the hash in ctx. Signal a failure
@@ -359,6 +409,19 @@ static int deflate_obj_to_pack(struct bulk_checkin_packfile *state,
return 0;
}
+static int deflate_obj_to_pack_incore(struct bulk_checkin_packfile *state,
+ struct object_id *result_oid,
+ const void *buf, size_t size,
+ const char *path, enum object_type type,
+ unsigned flags)
+{
+ struct bulk_checkin_source source;
+
+ init_bulk_checkin_source_incore(&source, buf, size, path);
+
+ return deflate_obj_to_pack(state, result_oid, &source, type, 0, flags);
+}
+
static int deflate_blob_to_pack(struct bulk_checkin_packfile *state,
struct object_id *result_oid,
int fd, size_t size,
@@ -421,6 +484,18 @@ int index_blob_bulk_checkin(struct object_id *oid,
return status;
}
+int index_blob_bulk_checkin_incore(struct object_id *oid,
+ const void *buf, size_t size,
+ const char *path, unsigned flags)
+{
+ int status = deflate_obj_to_pack_incore(&bulk_checkin_packfile, oid,
+ buf, size, path, OBJ_BLOB,
+ flags);
+ if (!odb_transaction_nesting)
+ flush_bulk_checkin_packfile(&bulk_checkin_packfile);
+ return status;
+}
+
void begin_odb_transaction(void)
{
odb_transaction_nesting += 1;
diff --git a/bulk-checkin.h b/bulk-checkin.h
index aa7286a7b3..1b91daeaee 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -13,6 +13,10 @@ int index_blob_bulk_checkin(struct object_id *oid,
int fd, size_t size,
const char *path, unsigned flags);
+int index_blob_bulk_checkin_incore(struct object_id *oid,
+ const void *buf, size_t size,
+ const char *path, unsigned flags);
+
/*
* Tell the object database to optimize for adding
* multiple objects. end_odb_transaction must be called
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply related
* [PATCH v5 4/5] bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
From: Taylor Blau @ 2023-10-23 22:45 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1698101088.git.me@ttaylorr.com>
The remaining missing piece in order to teach the `merge-tree` builtin
how to write the contents of a merge into a pack is a function to index
tree objects into a bulk-checkin pack.
This patch implements that missing piece, which is a thin wrapper around
all of the functionality introduced in previous commits.
If and when Git gains support for a "compatibility" hash algorithm, the
changes to support that here will be minimal. The bulk-checkin machinery
will need to convert the incoming tree to compute its length under the
compatibility hash, necessary to reconstruct its header. With that
information (and the converted contents of the tree), the bulk-checkin
machinery will have enough to keep track of the converted object's hash
in order to update the compatibility mapping.
Within some thin wrapper around `deflate_obj_to_pack_incore()` (perhaps
`deflate_tree_to_pack_incore()`), the changes should be limited to
something like:
struct strbuf converted = STRBUF_INIT;
if (the_repository->compat_hash_algo) {
if (convert_object_file(&compat_obj,
the_repository->hash_algo,
the_repository->compat_hash_algo, ...) < 0)
die(...);
format_object_header_hash(the_repository->compat_hash_algo,
OBJ_TREE, size);
}
/* compute the converted tree's hash using the compat algorithm */
strbuf_release(&converted);
, assuming related changes throughout the rest of the bulk-checkin
machinery necessary to update the hash of the converted object, which
are likewise minimal in size.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
bulk-checkin.c | 12 ++++++++++++
bulk-checkin.h | 4 ++++
2 files changed, 16 insertions(+)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index b728210bc7..bd6151ba3c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -496,6 +496,18 @@ int index_blob_bulk_checkin_incore(struct object_id *oid,
return status;
}
+int index_tree_bulk_checkin_incore(struct object_id *oid,
+ const void *buf, size_t size,
+ const char *path, unsigned flags)
+{
+ int status = deflate_obj_to_pack_incore(&bulk_checkin_packfile, oid,
+ buf, size, path, OBJ_TREE,
+ flags);
+ if (!odb_transaction_nesting)
+ flush_bulk_checkin_packfile(&bulk_checkin_packfile);
+ return status;
+}
+
void begin_odb_transaction(void)
{
odb_transaction_nesting += 1;
diff --git a/bulk-checkin.h b/bulk-checkin.h
index 1b91daeaee..89786b3954 100644
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -17,6 +17,10 @@ int index_blob_bulk_checkin_incore(struct object_id *oid,
const void *buf, size_t size,
const char *path, unsigned flags);
+int index_tree_bulk_checkin_incore(struct object_id *oid,
+ const void *buf, size_t size,
+ const char *path, unsigned flags);
+
/*
* Tell the object database to optimize for adding
* multiple objects. end_odb_transaction must be called
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply related
* [PATCH v5 5/5] builtin/merge-tree.c: implement support for `--write-pack`
From: Taylor Blau @ 2023-10-23 22:45 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Eric W. Biederman, Jeff King, Junio C Hamano,
Patrick Steinhardt
In-Reply-To: <cover.1698101088.git.me@ttaylorr.com>
When using merge-tree often within a repository[^1], it is possible to
generate a relatively large number of loose objects, which can result in
degraded performance, and inode exhaustion in extreme cases.
Building on the functionality introduced in previous commits, the
bulk-checkin machinery now has support to write arbitrary blob and tree
objects which are small enough to be held in-core. We can use this to
write any blob/tree objects generated by ORT into a separate pack
instead of writing them out individually as loose.
This functionality is gated behind a new `--write-pack` option to
`merge-tree` that works with the (non-deprecated) `--write-tree` mode.
The implementation is relatively straightforward. There are two spots
within the ORT mechanism where we call `write_object_file()`, one for
content differences within blobs, and another to assemble any new trees
necessary to construct the merge. In each of those locations,
conditionally replace calls to `write_object_file()` with
`index_blob_bulk_checkin_incore()` or `index_tree_bulk_checkin_incore()`
depending on which kind of object we are writing.
The only remaining task is to begin and end the transaction necessary to
initialize the bulk-checkin machinery, and move any new pack(s) it
created into the main object store.
[^1]: Such is the case at GitHub, where we run presumptive "test merges"
on open pull requests to see whether or not we can light up the merge
button green depending on whether or not the presumptive merge was
conflicted.
This is done in response to a number of user-initiated events,
including viewing an open pull request whose last test merge is stale
with respect to the current base and tip of the pull request. As a
result, merge-tree can be run very frequently on large, active
repositories.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-merge-tree.txt | 4 ++
builtin/merge-tree.c | 5 ++
merge-ort.c | 42 +++++++++++----
merge-recursive.h | 1 +
t/t4301-merge-tree-write-tree.sh | 93 ++++++++++++++++++++++++++++++++
5 files changed, 136 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
index ffc4fbf7e8..9d37609ef1 100644
--- a/Documentation/git-merge-tree.txt
+++ b/Documentation/git-merge-tree.txt
@@ -69,6 +69,10 @@ OPTIONS
specify a merge-base for the merge, and specifying multiple bases is
currently not supported. This option is incompatible with `--stdin`.
+--write-pack::
+ Write any new objects into a separate packfile instead of as
+ individual loose objects.
+
[[OUTPUT]]
OUTPUT
------
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index a35e0452d6..218442ac9b 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -19,6 +19,7 @@
#include "tree.h"
#include "config.h"
#include "strvec.h"
+#include "bulk-checkin.h"
static int line_termination = '\n';
@@ -416,6 +417,7 @@ struct merge_tree_options {
int name_only;
int use_stdin;
struct merge_options merge_options;
+ int write_pack;
};
static int real_merge(struct merge_tree_options *o,
@@ -441,6 +443,7 @@ static int real_merge(struct merge_tree_options *o,
_("not something we can merge"));
opt.show_rename_progress = 0;
+ opt.write_pack = o->write_pack;
opt.branch1 = branch1;
opt.branch2 = branch2;
@@ -553,6 +556,8 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
N_("specify a merge-base for the merge")),
OPT_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
N_("option for selected merge strategy")),
+ OPT_BOOL(0, "write-pack", &o.write_pack,
+ N_("write new objects to a pack instead of as loose")),
OPT_END()
};
diff --git a/merge-ort.c b/merge-ort.c
index 3653725661..523577d71e 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -48,6 +48,7 @@
#include "tree.h"
#include "unpack-trees.h"
#include "xdiff-interface.h"
+#include "bulk-checkin.h"
/*
* We have many arrays of size 3. Whenever we have such an array, the
@@ -2108,10 +2109,19 @@ static int handle_content_merge(struct merge_options *opt,
if ((merge_status < 0) || !result_buf.ptr)
ret = error(_("failed to execute internal merge"));
- if (!ret &&
- write_object_file(result_buf.ptr, result_buf.size,
- OBJ_BLOB, &result->oid))
- ret = error(_("unable to add %s to database"), path);
+ if (!ret) {
+ ret = opt->write_pack
+ ? index_blob_bulk_checkin_incore(&result->oid,
+ result_buf.ptr,
+ result_buf.size,
+ path, 1)
+ : write_object_file(result_buf.ptr,
+ result_buf.size,
+ OBJ_BLOB, &result->oid);
+ if (ret)
+ ret = error(_("unable to add %s to database"),
+ path);
+ }
free(result_buf.ptr);
if (ret)
@@ -3597,7 +3607,8 @@ static int tree_entry_order(const void *a_, const void *b_)
b->string, strlen(b->string), bmi->result.mode);
}
-static int write_tree(struct object_id *result_oid,
+static int write_tree(struct merge_options *opt,
+ struct object_id *result_oid,
struct string_list *versions,
unsigned int offset,
size_t hash_size)
@@ -3631,8 +3642,14 @@ static int write_tree(struct object_id *result_oid,
}
/* Write this object file out, and record in result_oid */
- if (write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid))
+ ret = opt->write_pack
+ ? index_tree_bulk_checkin_incore(result_oid,
+ buf.buf, buf.len, "", 1)
+ : write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid);
+
+ if (ret)
ret = -1;
+
strbuf_release(&buf);
return ret;
}
@@ -3797,8 +3814,8 @@ static int write_completed_directory(struct merge_options *opt,
*/
dir_info->is_null = 0;
dir_info->result.mode = S_IFDIR;
- if (write_tree(&dir_info->result.oid, &info->versions, offset,
- opt->repo->hash_algo->rawsz) < 0)
+ if (write_tree(opt, &dir_info->result.oid, &info->versions,
+ offset, opt->repo->hash_algo->rawsz) < 0)
ret = -1;
}
@@ -4332,9 +4349,13 @@ static int process_entries(struct merge_options *opt,
fflush(stdout);
BUG("dir_metadata accounting completely off; shouldn't happen");
}
- if (write_tree(result_oid, &dir_metadata.versions, 0,
+ if (write_tree(opt, result_oid, &dir_metadata.versions, 0,
opt->repo->hash_algo->rawsz) < 0)
ret = -1;
+
+ if (opt->write_pack)
+ end_odb_transaction();
+
cleanup:
string_list_clear(&plist, 0);
string_list_clear(&dir_metadata.versions, 0);
@@ -4878,6 +4899,9 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
*/
strmap_init(&opt->priv->conflicts);
+ if (opt->write_pack)
+ begin_odb_transaction();
+
trace2_region_leave("merge", "allocate/init", opt->repo);
}
diff --git a/merge-recursive.h b/merge-recursive.h
index 3d3b3e3c29..5c5ff380a8 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -48,6 +48,7 @@ struct merge_options {
unsigned renormalize : 1;
unsigned record_conflict_msgs_as_headers : 1;
const char *msg_header_prefix;
+ unsigned write_pack : 1;
/* internal fields used by the implementation */
struct merge_options_internal *priv;
diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
index b2c8a43fce..d2a8634523 100755
--- a/t/t4301-merge-tree-write-tree.sh
+++ b/t/t4301-merge-tree-write-tree.sh
@@ -945,4 +945,97 @@ test_expect_success 'check the input format when --stdin is passed' '
test_cmp expect actual
'
+packdir=".git/objects/pack"
+
+test_expect_success 'merge-tree can pack its result with --write-pack' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+
+ # base has lines [3, 4, 5]
+ # - side adds to the beginning, resulting in [1, 2, 3, 4, 5]
+ # - other adds to the end, resulting in [3, 4, 5, 6, 7]
+ #
+ # merging the two should result in a new blob object containing
+ # [1, 2, 3, 4, 5, 6, 7], along with a new tree.
+ test_commit -C repo base file "$(test_seq 3 5)" &&
+ git -C repo branch -M main &&
+ git -C repo checkout -b side main &&
+ test_commit -C repo side file "$(test_seq 1 5)" &&
+ git -C repo checkout -b other main &&
+ test_commit -C repo other file "$(test_seq 3 7)" &&
+
+ find repo/$packdir -type f -name "pack-*.idx" >packs.before &&
+ tree="$(git -C repo merge-tree --write-pack \
+ refs/tags/side refs/tags/other)" &&
+ blob="$(git -C repo rev-parse $tree:file)" &&
+ find repo/$packdir -type f -name "pack-*.idx" >packs.after &&
+
+ test_must_be_empty packs.before &&
+ test_line_count = 1 packs.after &&
+
+ git show-index <$(cat packs.after) >objects &&
+ test_line_count = 2 objects &&
+ grep "^[1-9][0-9]* $tree" objects &&
+ grep "^[1-9][0-9]* $blob" objects
+'
+
+test_expect_success 'merge-tree can write multiple packs with --write-pack' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+
+ git config pack.packSizeLimit 512 &&
+
+ test_seq 512 >f &&
+
+ # "f" contains roughly ~2,000 bytes.
+ #
+ # Each side ("foo" and "bar") adds a small amount of data at the
+ # beginning and end of "base", respectively.
+ git add f &&
+ test_tick &&
+ git commit -m base &&
+ git branch -M main &&
+
+ git checkout -b foo main &&
+ {
+ echo foo && cat f
+ } >f.tmp &&
+ mv f.tmp f &&
+ git add f &&
+ test_tick &&
+ git commit -m foo &&
+
+ git checkout -b bar main &&
+ echo bar >>f &&
+ git add f &&
+ test_tick &&
+ git commit -m bar &&
+
+ find $packdir -type f -name "pack-*.idx" >packs.before &&
+ # Merging either side should result in a new object which is
+ # larger than 1M, thus the result should be split into two
+ # separate packs.
+ tree="$(git merge-tree --write-pack \
+ refs/heads/foo refs/heads/bar)" &&
+ blob="$(git rev-parse $tree:f)" &&
+ find $packdir -type f -name "pack-*.idx" >packs.after &&
+
+ test_must_be_empty packs.before &&
+ test_line_count = 2 packs.after &&
+ for idx in $(cat packs.after)
+ do
+ git show-index <$idx || return 1
+ done >objects &&
+
+ # The resulting set of packs should contain one copy of both
+ # objects, each in a separate pack.
+ test_line_count = 2 objects &&
+ grep "^[1-9][0-9]* $tree" objects &&
+ grep "^[1-9][0-9]* $blob" objects
+
+ )
+'
+
test_done
--
2.42.0.425.g963d08ddb3.dirty
^ permalink raw reply related
* Re: [PATCH v2] doc/git-bisect: clarify `git bisect run` syntax
From: Javier Mora @ 2023-10-23 22:53 UTC (permalink / raw)
To: Eric Sunshine; +Cc: cousteau via GitGitGadget, git, Patrick Steinhardt
In-Reply-To: <CAPig+cQuBwzaG7ZssGUY6k8wf8pcGZHAGLnbRy579uTPMKqwKQ@mail.gmail.com>
> the patch subject becomes a bit outdated with this addition.
Right; I wanted to change it to something like "clarify `git bisect
run` syntax and other minor changes" but wanted to keep the title
concise.
I guess I could change it to just "clarify `git bisect` syntax" though
remove the "run").
> the following two lines are already referencing placeholders
> <term-new> and <term-old>
That's why I added it; that `(bad|new|<term-new>)` felt a bit awkward
with no previous explanation of what <term-new> was.
> ...now we have an inconsistency again since this text just uses the
> generic <term>. However, I haven't convinced myself that we need to
> care about this inconsistency.
I thought about that, but in THAT case it wasn't necessary because
<term-new> and <term-old> are never used there (and I wanted to avoid
making -h too long). But it's true that it feels inconsistent; I may
add it just for the sake of consistency.
Overall, maybe I should leave that change to a separate patch, even if
it's a minor correction. (This made more sense when I had in mind the
plan to move everything from description to synopsis so I would need
to touch all those lines anyway.) The changes will be compatible
anyway (they're far away enough to not cause merge conflicts). What
do you think?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox