* Reporting Bug in Git Version Control System
From: Yash Jain @ 2016-10-24 14:28 UTC (permalink / raw)
To: git
Hello,
I have two accounts on github("yj291197" and "yaki29").
Both the accounts have different gmail IDs("yj291197@gmail.com" and
"yashjain.lnm@gmail.com" respectively) but same passwords.
I used to use git for "yj291197" account and a few days earlier I made
this new account and used git commit to commit on "yaki29" but it
appeared as "yj291197" committed on "yaki29's" repo.
Then I pulled a request of that commit then it appeared "yaki29"
pulled a request with a commit of "yj291197".
And during this whole session I was signed in as "yaki29" on github.com .
Please reply ....
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-24 14:02 UTC (permalink / raw)
To: Max Horn
Cc: Junio C Hamano, git, Stefan Beller, Jeff King,
Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <DAD768D3-5558-49DE-9FDD-E46F17933ECE@quendi.de>
Hi Max,
On Mon, 24 Oct 2016, Max Horn wrote:
> > On 23 Oct 2016, at 11:54, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Sat, 22 Oct 2016, Junio C Hamano wrote:
> >
> [...]
>
> >> There isn't enough time to include this topic in the upcoming release
> >> within the current https://tinyurl.com/gitCal calendar, however,
> >> which places the final on Nov 11th.
> >
> > More is the pity.
> >
> > Thank you, though, for being upfront with me. I will shift my focus to
> > tasks that require my attention more urgently, then.
>
> Junio did go on, though:
>
> >> I am wondering if it makes sense to delay 2.11 by moving the final
> >> by 4 weeks to Dec 9th.
>
> I was reading this as an offer to delay things to accommodate the
> integration your work into 2.11. I.e. "within the current plan, there is
> no time for this, but we could adjust the plan". But maybe I am
> misinterpreting?
There is no indication that the rebase--helper patches would make it into
2.11 even with four more weeks.
I will now focus on other things that I postponed in favor of the
interactive rebase patches. In fact, I *have* to focus on some quite
pressing tasks that I neglected over those patches.
It's not like the process would magically improve just because a release
date is pushed. To the contrary, pushing the release date to allow for the
rebase--helper to be included may very well have the counterintuitive
effect of delaying things beyond even that pushed date "because there is
now so much time left" (until there isn't). It's a variation of
[Parkinson's Law](https://en.wikipedia.org/wiki/Parkinson%27s_law) ;-)
Anyway, back to work,
Dscho
^ permalink raw reply
* [RFH] limiting ref advertisements
From: Jeff King @ 2016-10-24 13:29 UTC (permalink / raw)
To: git
I'm looking into the oft-discussed idea of reducing the size of ref
advertisements by having the client say "these are the refs I'm
interested in". Let's set aside the protocol complexities for a
moment and imagine we magically have some way to communicate a set of
patterns to the server.
What should those patterns look like?
I had hoped that we could keep most of the pattern logic on the
client-side. Otherwise we risk incompatibilities between how the client
and server interpret a pattern. I had also hoped we could do some kind
of prefix-matching, which would let the server look only at the
interesting bits of the ref tree (so if you don't care about
refs/changes, and the server has some ref storage that is hierarchical,
they can literally get away without opening that sub-tree).
The patch at the end of this email is what I came up with in that
direction. It obviously won't compile without the twenty other patches
implementing transport->advertise_prefixes, but it gives you a sense of
what I'm talking about.
Unfortunately it doesn't work in all cases, because refspec sources may
be unqualified. If I ask for:
git fetch $remote master:foo
then we have to actually dwim-resolve "master" from the complete list of
refs we get from the remote. It could be "refs/heads/master",
"refs/tags/master", etc. Worse, it could be "refs/master". In that case,
at least, I think we are OK because we avoid advertising refs directly
below "refs/" in the first place. But if you have a slash, like:
git fetch $remote jk/foo
then that _could_ be "refs/jk/foo". Likewise, we cannot even optimize
the common case of a fully-qualified ref, like "refs/heads/foo". If it
exists, we obviously want to use that. But if it doesn't, then it
could be refs/something-else/refs/heads/foo. That's unlikely, but it
_does_ work now, and optimizing the advertisement would break it.
So it seems like left-anchoring the refspecs can never be fully correct.
We can communicate "master" to the server, who can then look at every
ref it would advertise and ask "could this be called master"? But it
will be setting in stone the set of "could this be" patterns. Granted,
those haven't changed much over the history of git, but it seems awfully
fragile.
In an ideal world the client and server would negotiate to come to some
agreement on the patterns being used. But as we are bolting this onto
the existing protocol, I was really trying to do it without introducing
an extra capabilities phase or extra round-trips. I.e., something like
David Turner's "stick the refspec in the HTTP query parameters" trick,
but working everywhere[1].
Clever ideas?
-Peff
[1] I do have working patches to pass these "early capabilities"
everywhere, but they're still somewhat rough. I got it to the point
where I could flip the default to "on" to see what breaks. That's
not something we'd want to do for real, but is good for running the
test suite to uncover issues like this one.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7c10d70092..3a2585ffd7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -302,6 +302,33 @@ static void find_non_local_tags(struct transport *transport,
string_list_clear(&remote_refs, 0);
}
+static void add_advertise_prefixes(struct transport *transport,
+ const struct refspec *refs, int nr)
+{
+ struct argv_array *list = &transport->advertise_prefixes;
+ int i;
+
+ for (i = 0; i < nr; i++) {
+ const struct refspec *rs = &refs[i];
+ size_t len;
+
+ if (!rs->pattern)
+ argv_array_push(list, rs->src);
+ else if (strip_suffix(rs->src, "/*", &len))
+ argv_array_pushf(list, "%.*s", (int)len, rs->src);
+ else {
+ /*
+ * This refspec is too complex for us to communicate;
+ * not only do we skip it, but we must avoid
+ * communicating any prefixes, since we need to see
+ * all refs.
+ */
+ transport->ignore_advertise_prefixes = 1;
+ return;
+ }
+ }
+}
+
static struct ref *get_ref_map(struct transport *transport,
struct refspec *refspecs, int refspec_count,
int tags, int *autotags)
@@ -314,12 +341,18 @@ static struct ref *get_ref_map(struct transport *transport,
/* opportunistically-updated references: */
struct ref *orefs = NULL, **oref_tail = &orefs;
- const struct ref *remote_refs = transport_get_remote_refs(transport);
+ const struct ref *remote_refs;
+
+ if (tags == TAGS_SET || (tags == TAGS_DEFAULT && *autotags))
+ add_advertise_prefixes(transport, tag_refspec, 1);
if (refspec_count) {
struct refspec *fetch_refspec;
int fetch_refspec_nr;
+ add_advertise_prefixes(transport, refspecs, refspec_count);
+ remote_refs = transport_get_remote_refs(transport);
+
for (i = 0; i < refspec_count; i++) {
get_fetch_map(remote_refs, &refspecs[i], &tail, 0);
if (refspecs[i].dst && refspecs[i].dst[0])
@@ -373,6 +406,17 @@ static struct ref *get_ref_map(struct transport *transport,
(remote->fetch_refspec_nr ||
/* Note: has_merge implies non-NULL branch->remote_name */
(has_merge && !strcmp(branch->remote_name, remote->name)))) {
+
+ add_advertise_prefixes(transport, remote->fetch,
+ remote->fetch_refspec_nr);
+ if (has_merge && !strcmp(branch->remote_name, remote->name)) {
+ int i;
+ for (i = 0; i < branch->merge_nr; i++)
+ add_advertise_prefixes(transport, branch->merge[i], 1);
+ }
+
+ remote_refs = transport_get_remote_refs(transport);
+
for (i = 0; i < remote->fetch_refspec_nr; i++) {
get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
if (remote->fetch[i].dst &&
@@ -393,6 +437,8 @@ static struct ref *get_ref_map(struct transport *transport,
!strcmp(branch->remote_name, remote->name))
add_merge_config(&ref_map, remote_refs, branch, &tail);
} else {
+ argv_array_push(&transport->advertise_prefixes, "HEAD");
+ remote_refs = transport_get_remote_refs(transport);
ref_map = get_remote_ref(remote_refs, "HEAD");
if (!ref_map)
die(_("Couldn't find remote ref HEAD"));
^ permalink raw reply related
* Re: [PATCH] hex: use unsigned index for ring buffer
From: Jeff King @ 2016-10-24 13:00 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <fb816dd5-8fb9-c6a6-2ec2-9ea4dddfdb26@web.de>
On Sun, Oct 23, 2016 at 07:57:30PM +0200, René Scharfe wrote:
> > > Hard to trigger, but probably even harder to diagnose once someone
> > > somehow manages to do it on some uncommon architecture.
> >
> > Indeed. If we are worried about overflow, we would also want to assume
> > that it wraps at a multiple of 4, but that is probably a sane
> > assumption.
>
> Hmm, I can't think of a way to violate this assumption except with unsigned
> integers that are only a single bit wide. That would be a weird machine.
> Are there other possibilities?
No, I don't think so. I don't recall offhand whether the C standard
allows integers that are not powers of 2. But if it does, and somebody
develops such a platform, I have very little sympathy.
My comment was mostly "this is the only other restriction I can think
of, and it is crazy".
> > You could also write the second line like:
> >
> > bufno %= ARRAY_SIZE(hexbuffer);
> >
> > which is less magical (right now the set of buffers must be a power of
> > 2). I expect the compiler could turn that into a bitmask itself.
>
> Expelling magic is a good idea. And indeed, at least gcc, clang and icc on
> x86-64 are smart enough to use an AND instead of dividing
> (https://godbolt.org/g/rFPpzF).
>
> But gcc also adds a sign extension (cltq/cdqe) if we store the truncated
> value, unlike the other two compilers. I don't see why -- the bit mask
> operation enforces a value between 0 and 3 (inclusive) and the upper bits of
> eax are zeroed automatically, so the cltq is effectively a noop.
>
> Using size_t gets us rid of the extra instruction and is the right type
> anyway. It would suffice on its own, hmm..
Yeah, I had assumed you would also switch to some form of unsigned type
either way.
> > I'm fine with any of the options. I guess you'd want a similar patch for
> > find_unique_abbrev on top of jk/no-looking-at-dotgit-outside-repo.
>
> Actually I'd want you to want to amend your series yourself. ;) Maybe I can
> convince Coccinelle to handle that issue for us.
I thought that series was in "next" already, but I see it isn't yet. I'd
still wait until the sha1_to_hex() solution settles, and then copy it.
> And there's also path.c::get_pathname(). That's enough cases to justify
> adding a macro, I'd say:
> [...]
> +#define NEXT_RING_ITEM(array, index) \
> + (array)[(index) = ((index) + 1) % ARRAY_SIZE(array)]
> +
I dunno. It hides a lot of magic without saving a lot of lines in the
caller, and the callers have to make sure "array" is an array and that
"index" is unsigned.
E.g., in this code:
> @@ -24,8 +24,8 @@ static struct strbuf *get_pathname(void)
> static struct strbuf pathname_array[4] = {
> STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
> };
> - static int index;
> - struct strbuf *sb = &pathname_array[3 & ++index];
> + static size_t index;
> + struct strbuf *sb = &NEXT_RING_ITEM(pathname_array, index);
> strbuf_reset(sb);
> return sb;
> }
The truly ugly part is the repeated STRBUF_INIT. :)
I think it would be preferable to just fix it inline in each place.
-Peff
^ permalink raw reply
* Re: [PATCH] Allow stashes to be referenced by index only
From: Jeff King @ 2016-10-24 12:54 UTC (permalink / raw)
To: Aaron and Ashley Watson
Cc: git, Jon Seymour, David Caldwell, Øystein Walle,
Ævar Arnfjörð Bjarmason, David Aguilar,
Alex Henrie
In-Reply-To: <CAB0+k9JrX7Ax26HfTEgoSyj02szFyHLayqTyW6KPuxVvXBOEOw@mail.gmail.com>
On Sun, Oct 23, 2016 at 01:41:25PM -0400, Aaron and Ashley Watson wrote:
> > But what's going on here? Why did we bother running rev-parse earlier if
> > we don't actually use the value of REV?
> >
> > You mentioned tweaking it to fix a broken test, and indeed, just using
> > $REV here breaks a few tests in t3903.
> >
> > Offhand, I do not see anything wrong with pulling the non-option values
> > out in the loop. But in that case I think the assignment of REV can just
> > go away completely.
> >
>
> The only reason for REV to remain is to preserve the error message seen with
> the previous behavior. Perhaps it would be better to instead move the
> assignment
> of REV to the only place it is still used: the error message when multiple
> arguments were detected.
Ah, thanks, I missed that use. We suppress stderr, so we're literally
just getting the set of revs there. But that should match what we have
in ARGV anyway (after all, $ARGV is where we decided we had too many
revs, and what we'll feed to rev-parse to get the sha1).
So I wonder if:
Too many revisions specified: $ARGV
would be more appropriate (at which point you can probably just continue
to call it $REV).
> I'm not sure of the next steps in the process of submitting a patch.
> Should I submit a new patch by replying to this email, or is using git
> send-email to create a new mail thread better?
Generally re-rolls of a patch are done as replies to the original. Gmail
is bad about corrupting whitespace, though, so you can't just reply and
paste the patch in there. You can use ask git-send-email to continue the
thread, though:
mid=1473378397-22453-1-git-send-email-watsona4@gmail.com
git send-email --in-reply-to=$mid ...other options...
You might also want to cc the people involved in the earlier discussion.
The public-inbox archive gives a customized full send-email command for
each message to make it easy.
http://public-inbox.org/git/1473378397-22453-1-git-send-email-watsona4@gmail.com/
-Peff
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Max Horn @ 2016-10-24 12:24 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, git, Stefan Beller, Jeff King,
Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <alpine.DEB.2.20.1610231151140.3264@virtualbox>
Hi Dscho,
> On 23 Oct 2016, at 11:54, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> Hi Junio,
>
> On Sat, 22 Oct 2016, Junio C Hamano wrote:
>
[...]
>> There isn't enough time to include this topic in the upcoming
>> release within the current https://tinyurl.com/gitCal calendar,
>> however, which places the final on Nov 11th.
>
> More is the pity.
>
> Thank you, though, for being upfront with me. I will shift my focus to
> tasks that require my attention more urgently, then.
Junio did go on, though:
>> I am wondering if it makes sense to delay 2.11 by moving the final
>> by 4 weeks to Dec 9th.
I was reading this as an offer to delay things to accommodate the integration your work into 2.11. I.e. "within the current plan, there is no time for this, but we could adjust the plan". But maybe I am misinterpreting?
Cheers,
Max
^ permalink raw reply
* [PATCH 4/4] commit: don't be fooled by ita entries when creating initial commit
From: Nguyễn Thái Ngọc Duy @ 2016-10-24 10:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161024104222.31128-1-pclouds@gmail.com>
ita entries are dropped at tree generation phase. If the entire index
consists of just ita entries, the result would be a a commit with no
entries, which should be caught unless --allow-empty is specified. The
test "!!active_nr" is not sufficient to catch this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/commit.c | 11 ++++++++---
t/t2203-add-intent.sh | 10 ++++++++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index fe8694d..42732ba 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -894,9 +894,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (amend)
parent = "HEAD^1";
- if (get_sha1(parent, sha1))
- commitable = !!active_nr;
- else {
+ if (get_sha1(parent, sha1)) {
+ int i, ita_nr = 0;
+
+ for (i = 0; i < active_nr; i++)
+ if (ce_intent_to_add(active_cache[i]))
+ ita_nr++;
+ commitable = active_nr - ita_nr > 0;
+ } else {
/*
* Unless the user did explicitly request a submodule
* ignore mode by passing a command line option we do
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 8652a96..84a9028 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -129,6 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
)
'
+test_expect_success 'commit: ita entries ignored in empty intial commit check' '
+ git init empty-intial-commit &&
+ (
+ cd empty-intial-commit &&
+ : >one &&
+ git add -N one &&
+ test_must_fail git commit -m nothing-new-here
+ )
+'
+
test_expect_success 'commit: ita entries ignored in empty commit check' '
git init empty-subsequent-commit &&
(
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 3/4] commit: fix empty commit creation when there's no changes but ita entries
From: Nguyễn Thái Ngọc Duy @ 2016-10-24 10:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161024104222.31128-1-pclouds@gmail.com>
If i-t-a entries are present and there is no change between the index
and HEAD i-t-a entries, index_differs_from() still returns "dirty, new
entries" (aka, the resulting commit is not empty), but cache-tree will
skip i-t-a entries and produce the exact same tree of current
commit.
index_differs_from() is supposed to catch this so we can abort
git-commit (unless --no-empty is specified). Update it to optionally
ignore i-t-a entries when doing a diff between the index and HEAD so
that it would return "no change" in this case and abort commit.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/commit.c | 2 +-
diff-lib.c | 4 +++-
diff.h | 2 +-
sequencer.c | 4 ++--
t/t2203-add-intent.sh | 11 +++++++++++
5 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index bb9f79b..fe8694d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -910,7 +910,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (ignore_submodule_arg &&
!strcmp(ignore_submodule_arg, "all"))
diff_flags |= DIFF_OPT_IGNORE_SUBMODULES;
- commitable = index_differs_from(parent, diff_flags);
+ commitable = index_differs_from(parent, diff_flags, 1);
}
}
strbuf_release(&committer_ident);
diff --git a/diff-lib.c b/diff-lib.c
index 27f1228..5244746 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -535,7 +535,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
return 0;
}
-int index_differs_from(const char *def, int diff_flags)
+int index_differs_from(const char *def, int diff_flags,
+ int ita_invisible_in_index)
{
struct rev_info rev;
struct setup_revision_opt opt;
@@ -547,6 +548,7 @@ int index_differs_from(const char *def, int diff_flags)
DIFF_OPT_SET(&rev.diffopt, QUICK);
DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
rev.diffopt.flags |= diff_flags;
+ rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
run_diff_index(&rev, 1);
if (rev.pending.alloc)
free(rev.pending.objects);
diff --git a/diff.h b/diff.h
index 68a6618..b171172 100644
--- a/diff.h
+++ b/diff.h
@@ -356,7 +356,7 @@ extern int diff_result_code(struct diff_options *, int);
extern void diff_no_index(struct rev_info *, int, const char **);
-extern int index_differs_from(const char *def, int diff_flags);
+extern int index_differs_from(const char *def, int diff_flags, int ita_invisible_in_index);
/*
* Fill the contents of the filespec "df", respecting any textconv defined by
diff --git a/sequencer.c b/sequencer.c
index eec8a60..b082635 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -469,7 +469,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
unborn = get_sha1("HEAD", head);
if (unborn)
hashcpy(head, EMPTY_TREE_SHA1_BIN);
- if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0))
+ if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD", 0, 0))
return error_dirty_index(opts);
}
discard_cache();
@@ -1064,7 +1064,7 @@ static int sequencer_continue(struct replay_opts *opts)
if (ret)
return ret;
}
- if (index_differs_from("HEAD", 0))
+ if (index_differs_from("HEAD", 0, 0))
return error_dirty_index(opts);
todo_list = todo_list->next;
return pick_commits(todo_list, opts);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 0e54f63..8652a96 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -129,5 +129,16 @@ test_expect_success 'cache-tree does skip dir that becomes empty' '
)
'
+test_expect_success 'commit: ita entries ignored in empty commit check' '
+ git init empty-subsequent-commit &&
+ (
+ cd empty-subsequent-commit &&
+ test_commit one &&
+ : >two &&
+ git add -N two &&
+ test_must_fail git commit -m nothing-new-here
+ )
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 2/4] diff: add --ita-[in]visible-in-index
From: Nguyễn Thái Ngọc Duy @ 2016-10-24 10:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161024104222.31128-1-pclouds@gmail.com>
The option --ita-invisible-in-index exposes the "ita_invisible_in_index"
diff flag to outside to allow easier experimentation with this new mode.
The "plan" is to make --ita-invisible-in-index default to keep consistent
behavior with 'status' and 'commit', but a bunch other commands like
'apply', 'merge', 'reset'.... need to be taken into consideration as well.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/diff-options.txt | 8 ++++++++
diff.c | 4 ++++
t/t2203-add-intent.sh | 4 +++-
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 7805a0c..0fdd53f 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -575,5 +575,13 @@ endif::git-format-patch[]
--line-prefix=<prefix>::
Prepend an additional prefix to every line of output.
+--ita-invisible-in-index::
+ By default entries added by "git add -N" appear as an existing
+ empty file in "git diff" and a new file in "git diff --cached".
+ This option makes the entry appear as a new file in "git diff"
+ and non-existent in "git diff --cached". This option could be
+ reverted with `--ita-visible-in-index`. Both options are
+ experimental and could be removed in future.
+
For more detailed explanation on these common options, see also
linkgit:gitdiffcore[7].
diff --git a/diff.c b/diff.c
index c6da383..e8e73f8 100644
--- a/diff.c
+++ b/diff.c
@@ -3923,6 +3923,10 @@ int diff_opt_parse(struct diff_options *options,
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
return parse_ws_error_highlight(options, arg);
+ else if (!strcmp(arg, "--ita-invisible-in-index"))
+ options->ita_invisible_in_index = 1;
+ else if (!strcmp(arg, "--ita-visible-in-index"))
+ options->ita_invisible_in_index = 0;
/* misc options */
else if (!strcmp(arg, "-z"))
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 2276e4e..0e54f63 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -57,7 +57,9 @@ test_expect_success 'i-t-a entry is simply ignored' '
git add -N nitfol &&
git commit -m second &&
test $(git ls-tree HEAD -- nitfol | wc -l) = 0 &&
- test $(git diff --name-only HEAD -- nitfol | wc -l) = 1
+ test $(git diff --name-only HEAD -- nitfol | wc -l) = 1 &&
+ test $(git diff --name-only --ita-invisible-in-index HEAD -- nitfol | wc -l) = 0 &&
+ test $(git diff --name-only --ita-invisible-in-index -- nitfol | wc -l) = 1
'
test_expect_success 'can commit with an unrelated i-t-a entry in index' '
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 0/4] nd/ita-empty-commit update
From: Nguyễn Thái Ngọc Duy @ 2016-10-24 10:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160928114348.1470-1-pclouds@gmail.com>
This version splits the old 1/3 into two, with better description in
1/4. The index_differs_from() also takes a flag to set/clear this new
flag instead of relying on has_ita_entries like the old 2/3.
The name ita-invisible-in-index is not perfect but I could not think
of any better. Another name could be diff-cached-ignores-ita, but
that's just half of what it does. The other half is diff-files-includes-ita...
Nguyễn Thái Ngọc Duy (4):
Subject: diff-lib: allow ita entries treated as "not yet exist in index"
diff: add --ita-[in]visible-in-index
commit: fix empty commit creation when there's no changes but ita entries
commit: don't be fooled by ita entries when creating initial commit
Documentation/diff-options.txt | 8 ++++++++
builtin/commit.c | 13 +++++++++----
diff-lib.c | 18 +++++++++++++++++-
diff.c | 4 ++++
diff.h | 3 ++-
sequencer.c | 4 ++--
t/t2203-add-intent.sh | 41 +++++++++++++++++++++++++++++++++++++++--
t/t7064-wtstatus-pv2.sh | 4 ++--
wt-status.c | 7 ++++++-
9 files changed, 89 insertions(+), 13 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH 1/4] diff-lib: allow ita entries treated as "not yet exist in index"
From: Nguyễn Thái Ngọc Duy @ 2016-10-24 10:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161024104222.31128-1-pclouds@gmail.com>
When comparing the index and the working tree to show which paths are
new, and comparing the tree recorded in the HEAD and the index to see if
committing the contents recorded in the index would result in an empty
commit, we would want the former comparison to say "these are new paths"
and the latter to say "there is no change" for paths that are marked as
intent-to-add.
We made a similar attempt at d95d728a ("diff-lib.c: adjust position of
i-t-a entries in diff", 2015-03-16), which redefined the semantics of
these two comparison modes globally, which was a disaster and had to be
reverted at 78cc1a54 ("Revert "diff-lib.c: adjust position of i-t-a
entries in diff"", 2015-06-23).
To make sure we do not repeat the same mistake, introduce a new internal
diffopt option so that this different semantics can be asked for only by
callers that ask it, while making sure other unaudited callers will get
the same comparison result.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff-lib.c | 14 ++++++++++++++
diff.h | 1 +
t/t2203-add-intent.sh | 16 +++++++++++++++-
t/t7064-wtstatus-pv2.sh | 4 ++--
wt-status.c | 7 ++++++-
5 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 3007c85..27f1228 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -214,6 +214,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
!is_null_oid(&ce->oid),
ce->name, 0);
continue;
+ } else if (revs->diffopt.ita_invisible_in_index &&
+ ce_intent_to_add(ce)) {
+ diff_addremove(&revs->diffopt, '+', ce->ce_mode,
+ EMPTY_BLOB_SHA1_BIN, 0,
+ ce->name, 0);
+ continue;
}
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
@@ -379,6 +385,14 @@ static void do_oneway_diff(struct unpack_trees_options *o,
struct rev_info *revs = o->unpack_data;
int match_missing, cached;
+ /* i-t-a entries do not actually exist in the index */
+ if (revs->diffopt.ita_invisible_in_index &&
+ idx && ce_intent_to_add(idx)) {
+ idx = NULL;
+ if (!tree)
+ return; /* nothing to diff.. */
+ }
+
/* if the entry is not checked out, don't examine work tree */
cached = o->index_only ||
(idx && ((idx->ce_flags & CE_VALID) || ce_skip_worktree(idx)));
diff --git a/diff.h b/diff.h
index ec76a90..68a6618 100644
--- a/diff.h
+++ b/diff.h
@@ -146,6 +146,7 @@ struct diff_options {
int dirstat_permille;
int setup;
int abbrev;
+ int ita_invisible_in_index;
/* white-space error highlighting */
#define WSEH_NEW 1
#define WSEH_CONTEXT 2
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 8f22c43..2276e4e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -5,10 +5,24 @@ test_description='Intent to add'
. ./test-lib.sh
test_expect_success 'intent to add' '
+ test_commit 1 &&
+ git rm 1.t &&
+ echo hello >1.t &&
echo hello >file &&
echo hello >elif &&
git add -N file &&
- git add elif
+ git add elif &&
+ git add -N 1.t
+'
+
+test_expect_success 'git status' '
+ git status --porcelain | grep -v actual >actual &&
+ cat >expect <<-\EOF &&
+ DA 1.t
+ A elif
+ A file
+ EOF
+ test_cmp expect actual
'
test_expect_success 'check result of "add -N"' '
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 3012a4d..e319fa2 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -246,8 +246,8 @@ test_expect_success 'verify --intent-to-add output' '
git add --intent-to-add intent1.add intent2.add &&
cat >expect <<-EOF &&
- 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent1.add
- 1 AM N... 000000 100644 100644 $_z40 $EMPTY_BLOB intent2.add
+ 1 .A N... 000000 000000 100644 $_z40 $_z40 intent1.add
+ 1 .A N... 000000 000000 100644 $_z40 $_z40 intent2.add
EOF
git status --porcelain=v2 >actual &&
diff --git a/wt-status.c b/wt-status.c
index 9a14658..05a7dcb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -437,7 +437,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
switch (p->status) {
case DIFF_STATUS_ADDED:
- die("BUG: worktree status add???");
+ d->mode_worktree = p->two->mode;
break;
case DIFF_STATUS_DELETED:
@@ -547,6 +547,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
setup_revisions(0, NULL, &rev, NULL);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
DIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);
+ rev.diffopt.ita_invisible_in_index = 1;
if (!s->show_untracked_files)
DIFF_OPT_SET(&rev.diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
if (s->ignore_submodule_arg) {
@@ -570,6 +571,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
setup_revisions(0, NULL, &rev, &opt);
DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ rev.diffopt.ita_invisible_in_index = 1;
if (s->ignore_submodule_arg) {
handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);
} else {
@@ -605,6 +607,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
if (!ce_path_match(ce, &s->pathspec, NULL))
continue;
+ if (ce_intent_to_add(ce))
+ continue;
it = string_list_insert(&s->change, ce->name);
d = it->util;
if (!d) {
@@ -911,6 +915,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
init_revisions(&rev, NULL);
DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
+ rev.diffopt.ita_invisible_in_index = 1;
memset(&opt, 0, sizeof(opt));
opt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH] hex: use unsigned index for ring buffer
From: René Scharfe @ 2016-10-23 17:57 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Junio C Hamano
In-Reply-To: <20161023091146.p2kmqvgwxdf77dnn@sigill.intra.peff.net>
Am 23.10.2016 um 11:11 schrieb Jeff King:
> On Sun, Oct 23, 2016 at 11:00:48AM +0200, René Scharfe wrote:
>
>> Overflow is defined for unsigned integers, but not for signed ones.
>> Make the ring buffer index in sha1_to_hex() unsigned to be on the
>> safe side.
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>> Hard to trigger, but probably even harder to diagnose once someone
>> somehow manages to do it on some uncommon architecture.
>
> Indeed. If we are worried about overflow, we would also want to assume
> that it wraps at a multiple of 4, but that is probably a sane
> assumption.
Hmm, I can't think of a way to violate this assumption except with
unsigned integers that are only a single bit wide. That would be a
weird machine. Are there other possibilities?
>> diff --git a/hex.c b/hex.c
>> index ab2610e..8c6c189 100644
>> --- a/hex.c
>> +++ b/hex.c
>> @@ -76,7 +76,7 @@ char *oid_to_hex_r(char *buffer, const struct object_id *oid)
>>
>> char *sha1_to_hex(const unsigned char *sha1)
>> {
>> - static int bufno;
>> + static unsigned int bufno;
>> static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
>> return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1);
>> }
>
> I wonder if just truncating bufno would be conceptually simpler (albeit
> longer):
>
> bufno++;
> bufno &= 3;
> return sha1_to_hex_r(hexbuffer[bufno], sha1);
>
> You could also write the second line like:
>
> bufno %= ARRAY_SIZE(hexbuffer);
>
> which is less magical (right now the set of buffers must be a power of
> 2). I expect the compiler could turn that into a bitmask itself.
Expelling magic is a good idea. And indeed, at least gcc, clang and icc
on x86-64 are smart enough to use an AND instead of dividing
(https://godbolt.org/g/rFPpzF).
But gcc also adds a sign extension (cltq/cdqe) if we store the truncated
value, unlike the other two compilers. I don't see why -- the bit mask
operation enforces a value between 0 and 3 (inclusive) and the upper
bits of eax are zeroed automatically, so the cltq is effectively a noop.
Using size_t gets us rid of the extra instruction and is the right type
anyway. It would suffice on its own, hmm..
> I'm fine with any of the options. I guess you'd want a similar patch for
> find_unique_abbrev on top of jk/no-looking-at-dotgit-outside-repo.
Actually I'd want you to want to amend your series yourself. ;) Maybe I
can convince Coccinelle to handle that issue for us.
And there's also path.c::get_pathname(). That's enough cases to justify
adding a macro, I'd say:
---
cache.h | 3 +++
hex.c | 4 ++--
path.c | 4 ++--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 05ecb88..8bb4918 100644
--- a/cache.h
+++ b/cache.h
@@ -555,6 +555,9 @@ extern int daemonize(void);
} \
} while (0)
+#define NEXT_RING_ITEM(array, index) \
+ (array)[(index) = ((index) + 1) % ARRAY_SIZE(array)]
+
/* Initialize and use the cache information */
struct lock_file;
extern int read_index(struct index_state *);
diff --git a/hex.c b/hex.c
index ab2610e..5e711b9 100644
--- a/hex.c
+++ b/hex.c
@@ -76,9 +76,9 @@ char *oid_to_hex_r(char *buffer, const struct
object_id *oid)
char *sha1_to_hex(const unsigned char *sha1)
{
- static int bufno;
+ static size_t bufno;
static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
- return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1);
+ return sha1_to_hex_r(NEXT_RING_ITEM(hexbuffer, bufno), sha1);
}
char *oid_to_hex(const struct object_id *oid)
diff --git a/path.c b/path.c
index a8e7295..60dba6a 100644
--- a/path.c
+++ b/path.c
@@ -24,8 +24,8 @@ static struct strbuf *get_pathname(void)
static struct strbuf pathname_array[4] = {
STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
};
- static int index;
- struct strbuf *sb = &pathname_array[3 & ++index];
+ static size_t index;
+ struct strbuf *sb = &NEXT_RING_ITEM(pathname_array, index);
strbuf_reset(sb);
return sb;
}
--
2.10.1
^ permalink raw reply related
* Re: RFC Failover url for fetches?
From: Jakub Narębski @ 2016-10-23 17:40 UTC (permalink / raw)
To: Junio C Hamano, Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqeg39bk40.fsf@gitster.mtv.corp.google.com>
W dniu 21.10.2016 o 21:03, Junio C Hamano pisze:
> Stefan Beller <sbeller@google.com> writes:
>
>> So when pushing it is possible to have multiple urls
>> (remote.<name>.url) configured.
>>
>> When fetching only the first configured url is considered.
>> Would it make sense to allow multiple urls and
>> try them one by one until one works?
>
> I do not think the two are related. Pushing to multiple is not "I
> want to update at least one of them" in the first place.
Push is/should be 'update all', fetch is/should be 'fetch any'.
I thought that multiple remote.<name>.url values provide this
fallback for fetch, but it looks like it isn't so...
>
> As to fetching from two or more places as "fallback", I am
> moderately negative to add it as a dumb feature that does nothing
> more than "My fetch from A failed, so let's blindly try it from B".
> I'd prefer to keep the "My fetch from A is failing" knowledge near
> the surface of end user's consciousness as a mechanism to pressure A
> to fix it--that way everybody who is fetching from A benefits.
> After all, doing "git remote add B" once (you'd need to tell the URL
> for B anyway to Git) and issuing "git fetch B" after seeing your
> regular "git fetch" fails once in a blue moon is not all that
> cumbersome, I would think.
One would need to configure fallback B remote to use the same
remote-branch namespace as remote A, if it is to be used as fallback,
I would think.
>
> Some people _may_ have objection based on A and B going out of sync,
> especially B may fall behind even yourself and cause non-ff errors,
> but I personally am not worried about that, because when somebody
> configures B as a fallback for A, there is an expectation that B is
> kept reasonably up to date. It would be a problem if some refs are
> expected to be constantly rewound at A (e.g. 'pu' in my tree) and
> configured to always force-fetch, though. A stale B would silently
> set such a branch in your repository back without failing.
Nb. there is also http-alternates mechanism... which nowadays doesn't
matter anyway, I would think.
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v1 10/19] read-cache: regenerate shared index if necessary
From: Ramsay Jones @ 2016-10-23 16:07 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161023092648.12086-11-chriscool@tuxfamily.org>
On 23/10/16 10:26, Christian Couder wrote:
> When writing a new split-index and there is a big number of cache
> entries in the split-index compared to the shared index, it is a
> good idea to regenerate the shared index.
>
> By default when the ratio reaches 20%, we will push back all
> the entries from the split-index into a new shared index file
> instead of just creating a new split-index file.
>
> The threshold can be configured using the
> "splitIndex.maxPercentChange" config variable.
>
> We need to adjust the existing tests in t1700 by setting
> "splitIndex.maxPercentChange" to 100 at the beginning of t1700,
> as the existing tests are assuming that the shared index is
> regenerated only when `git update-index --split-index` is used.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> read-cache.c | 33 ++++++++++++++++++++++++++++++++-
> t/t1700-split-index.sh | 1 +
> 2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index bb53823..a91fabe 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2216,6 +2216,36 @@ static int write_shared_index(struct index_state *istate,
> return ret;
> }
>
> +static const int default_max_percent_split_change = 20;
> +
> +int too_many_not_shared_entries(struct index_state *istate)
This function is a file-loacal symbol; could you please make it
a static function.
Thanks.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 28/36] attr: keep attr stack for each check
From: Ramsay Jones @ 2016-10-23 15:10 UTC (permalink / raw)
To: Stefan Beller, gitster; +Cc: git, bmwill, pclouds
In-Reply-To: <20161022233225.8883-29-sbeller@google.com>
On 23/10/16 00:32, Stefan Beller wrote:
> Instead of having a global attr stack, attach the stack to each check.
> This allows to use the attr in a multithreaded way.
>
>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> attr.c | 101 +++++++++++++++++++++++++++++++++++++++-----------------------
> attr.h | 4 ++-
> hashmap.h | 2 ++
> 3 files changed, 69 insertions(+), 38 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index 89ae155..b65437d 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -372,15 +372,17 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
> * .gitignore file and info/excludes file as a fallback.
> */
>
> -/* NEEDSWORK: This will become per git_attr_check */
> -static struct attr_stack {
> +struct attr_stack {
> struct attr_stack *prev;
> char *origin;
> size_t originlen;
> unsigned num_matches;
> unsigned alloc;
> struct match_attr **attrs;
> -} *attr_stack;
> +};
> +
> +struct hashmap all_attr_stacks;
> +int all_attr_stacks_init;
Mark symbols 'all_attr_stacks' and 'all_attr_stacks_init' with
the static keyword. (ie. these are file-local symbols).
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 17/36] attr: expose validity check for attribute names
From: Ramsay Jones @ 2016-10-23 15:07 UTC (permalink / raw)
To: Stefan Beller, gitster; +Cc: git, bmwill, pclouds
In-Reply-To: <20161022233225.8883-18-sbeller@google.com>
On 23/10/16 00:32, Stefan Beller wrote:
> From: Junio C Hamano <gitster@pobox.com>
>
> Export attr_name_valid() function, and a helper function that
> returns the message to be given when a given <name, len> pair
> is not a good name for an attribute.
>
> We could later update the message to exactly spell out what the
> rules for a good attribute name are, etc.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
[snip]
> +extern int attr_name_valid(const char *name, size_t namelen);
> +extern void invalid_attr_name_message(struct strbuf *, const char *, int);
> +
The symbol 'attr_name_valid()' is not used outside of attr.c, even
by the end of this series. Do you expect this function to be used
in any future series? (The export is deliberate and it certainly
seems like it should be part of the public interface, but ...)
In contrast, the 'invalid_attr_name_message()' function is called
from code in pathspec.c, which relies on 'git_attr_counted()' to
call 'attr_name_valid()' internally to check for validity. :-D
ATB,
Ramsay Jones
^ permalink raw reply
* Re: git clone --bare --origin incompatible?
From: Roman Neuhauser @ 2016-10-23 12:50 UTC (permalink / raw)
To: Andreas Schwab; +Cc: git
In-Reply-To: <87y41f8czg.fsf@linux-m68k.org>
# schwab@linux-m68k.org / 2016-10-23 14:29:55 +0200:
> On Okt 23 2016, Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
>
> > what is the reason clone --bare prohibits --origin?
> >
> > % git clone --bare -o fubar anything anywhere
> > fatal: --bare and --origin fubar options are incompatible.
>
> Since a bare clone maps remote branches directly to local branches,
> without any remote-tracking branches, --origin doesn't make sense.
is it going to break something though? i can still go and rename
the remote in the bare repo's config file afterwards.
--
roman
^ permalink raw reply
* Re: git clone --bare --origin incompatible?
From: Andreas Schwab @ 2016-10-23 12:29 UTC (permalink / raw)
To: Roman Neuhauser; +Cc: git
In-Reply-To: <20161023110338.GA1486@isis.sigpipe.cz>
On Okt 23 2016, Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
> what is the reason clone --bare prohibits --origin?
>
> % git clone --bare -o fubar anything anywhere
> fatal: --bare and --origin fubar options are incompatible.
Since a bare clone maps remote branches directly to local branches,
without any remote-tracking branches, --origin doesn't make sense.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* git clone --bare --origin incompatible?
From: Roman Neuhauser @ 2016-10-23 11:03 UTC (permalink / raw)
To: git
hello,
what is the reason clone --bare prohibits --origin?
% git clone --bare -o fubar anything anywhere
fatal: --bare and --origin fubar options are incompatible.
--
roman
^ permalink raw reply
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Duy Nguyen @ 2016-10-23 10:27 UTC (permalink / raw)
To: Jeff King; +Cc: Lukas Fleischer, Git Mailing List
In-Reply-To: <20161023013846.ct3olfabw2yhzio2@sigill.intra.peff.net>
On Sun, Oct 23, 2016 at 8:38 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Oct 23, 2016 at 08:23:01AM +0700, Duy Nguyen wrote:
>
>> I hit the same problem sometimes, but in my case sometimes I
>> accidentally do "git add" after "git add -p" and a configuration in
>> "git commit -a" won't help me. I'd prefer we could undo changes in
>> index instead. Something like reflog but for index.
>
> An index write always writes the whole file from scratch, so you really
> just need to save a copy of the old file. Perhaps something like:
>
> rm -f $GIT_DIR/index.old
> ln $GIT_DIR/index.old $GIT_DIR/index
> ... and then open $GIT_DIR/index.tmp ...
> ... and then rename(index.tmp, index) ...
>
> could do it cheaply. It's a little more complicated if you want to save
> a sequence of versions, and eventually would take a lot of space, but
> presumably a handful of saved indexes would be sufficient.
Yeah. I had something [1] like that but never sorted out the UI for it :(
> Another option would be an index format that journals, and you could
> potentially walk back the journal to a point. That seems like a much
> bigger change (and has weird layering, because deciding when to fold in
> the journal is usually a performance thing, but obviously this would
> have user-visible impact about how far back you could undo).
v2 [2] goes in this direction (but not a full blown COW, the journal
does not take part in any core operations of the index)
[1] https://public-inbox.org/git/%3C1375597720-13236-1-git-send-email-pclouds@gmail.com%3E/
[2] https://public-inbox.org/git/1375966270-10968-1-git-send-email-pclouds@gmail.com/
--
Duy
^ permalink raw reply
* Re: [PATCH 2/3] submodule tests: replace cloning from . by "$(pwd)"
From: Johannes Sixt @ 2016-10-23 10:14 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker, Jonathan Nieder
In-Reply-To: <CAGZ79kaukGh2ynkOQcF=skzxTMYr8CFRyGJw6FEmNsTAcaG_VQ@mail.gmail.com>
Am 22.10.2016 um 22:46 schrieb Stefan Beller:
> I have looked into it again, and by now I think the bug is a feature,
> actually.
>
> Consider this:
>
> git clone . super
> git -C super submodule add ../submodule
> # we thought the previous line is buggy
> git clone super super-clone
At this point, we *should* have this if there were no bugs (at least
that is my assumption):
/tmp
!
+ submodule <- submodule's remote repo
!
+ foo <- we are here (.), super's remote repo
!
+ super <- remote.origin.url=/tmp/foo/.
!
+ submodule <- remote.origin.url=/tmp/foo/./../submodule
submodule.submodule.url=../submodule
When I test this, 'git submodule add' fails:
foo@master> git -C super submodule add ../submodule
fatal: repository '/tmp/foo/submodule' does not exist
fatal: clone of '/tmp/foo/submodule' into submodule path
'/tmp/foo/super/submodule' failed
> Now in the super-clone the ../submodule is the correct
> relative url, because the url where we cloned from doesn't
> end in /.
I do not understand why this would be relevant. The question is not how
the submodule's remote URL ends, but how the submodule's remote URL is
constructed from the super-project's URL and the relative path specified
for 'git submodule add'.
Whether ../submodule or ./submodule is the correct relative URL depends
on where the origin of the submodule is located relative to the origin
of the super-project. In the above example, it is ../submodule. However,
the error message tells us that git looked in /tmp/foo/submodule, which
looks like the /. bug!
I do not understand where you see a feature here. What am I missing?
-- Hannes
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-23 9:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stefan Beller, Jeff King, Jakub Narębski, Johannes Sixt,
Ramsay Jones
In-Reply-To: <alpine.DEB.2.20.1610231151140.3264@virtualbox>
Hi Junio,
On Sun, 23 Oct 2016, Johannes Schindelin wrote:
> On Sat, 22 Oct 2016, Junio C Hamano wrote:
>
> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >
> > > This patch series marks the '4' in the countdown to speed up rebase -i
> > > by implementing large parts in C (read: there will be three more patch
> > > series after that before the full benefit hits git.git: sequencer-i,
> > > rebase--helper and rebase-i-extra).
> > > ...
> > > It would be *really* nice if we could get this patch series at least
> > > into `next` soon, as it gets late and later for the rest of the
> > > patches to make it into `master` in time for v2.11 (and it is not for
> > > lack of trying on my end...).
> >
> > This "countdown 4" step can affect cherry-pick and revert, even
Oh, I forgot to comment on this tidbit of your mail, sorry.
This *is* the countdown 4, as the remaining 3 patch series depend on each
other in the order I sent them out.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-23 9:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stefan Beller, Jeff King, Jakub Narębski, Johannes Sixt,
Ramsay Jones
In-Reply-To: <xmqqinsk8g1b.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sat, 22 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This patch series marks the '4' in the countdown to speed up rebase -i
> > by implementing large parts in C (read: there will be three more patch
> > series after that before the full benefit hits git.git: sequencer-i,
> > rebase--helper and rebase-i-extra).
> > ...
> > It would be *really* nice if we could get this patch series at least
> > into `next` soon, as it gets late and later for the rest of the
> > patches to make it into `master` in time for v2.11 (and it is not for
> > lack of trying on my end...).
>
> This "countdown 4" step can affect cherry-pick and revert, even
> though we were careful to review changes to the sequencer.c code.
As I pointed out in another mail in this thread: we should not fall into
the trap of overrating review.
In the case of the rebase--helper patches, so far the review mainly
resulted in more work for me (having to change spellings elsewhere, for
example), not in improving the changes I intended to introduce into
git.git's code.
Sure, there has been the occasional improvement, but it certainly feels as
if I spent about 80% of the work after each -v1 iteration on things that
have positively nothing at all to do with accelerating rebase -i.
> I prefer to cook it in 'next' sufficiently long to ensure that we hear
> feedbacks from non-Windows users if there is any unexpected breakage.
FWIW I am using the same patches not only on Windows but also in my Linux
VM.
> There isn't enough time to include this topic in the upcoming
> release within the current https://tinyurl.com/gitCal calendar,
> however, which places the final on Nov 11th.
More is the pity.
Thank you, though, for being upfront with me. I will shift my focus to
tasks that require my attention more urgently, then.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-23 9:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqqinslbl5t.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 21 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> I still do not understand (note that I am not saying "I do not
> accept"--acceptance or rejection happens after an understandable
> explanation is given, and "do not understand" means no such
> explanation has been given yet) your justification behind adding a
> technical debt to reimplement the author-script parser and not
> sharing it with "git am" in 13/27.
At this point, I am most of all reluctant to introduce such a huge change,
which surely would introduce a regression.
This is what happened a couple of times to me, most recently with the
hide-dot-gitdir patch series that worked flawlessly for years, had to be
dramatically changed during review to enter git.git, and introduced the
major regression that `core.hideDotFiles = gitDirOnly` was broken.
The lesson I learned: review should not be valued more than the test of
time. This lesson has been reinforced by all the regressions that have not
been caught by review nor the test suite running on Linux only.
It would be a different matter if I still had the cross-validator in place
(which I did when I sent out v1 of this patch series) and tons of time to
spend on accommodating your wishes, however I may disagree with them. And
in this instance, I thought I made clear that I disagree, and why:
Internally, git-am and git-rebase-i handle the author-script very
differently. That may change at some stage in the future, and it would be
a good time then and there to take care of unifying this code. Currently,
not so much, as the only excuse to use the same parser would be that they
both read the same file, while they have to do very different things with
the parsed output (in fact, your suggestion would ask the parser in the
sequencer to rip apart the information into key/value pairs, only to
re-glue them back together when they are used as the environment variables
as which rebase-i treats the contents of the author-script file).
So no, at this point I am not willing to risk introducing breakages in
code that has been proven to work in practice.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v5 22/27] sequencer: teach write_message() to append an optional LF
From: Johannes Schindelin @ 2016-10-23 9:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <xmqqmvhxbljm.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 21 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > This commit prepares for future callers that will have a pointer/length
> > to some text to be written that lacks an LF, yet an LF is desired.
> > Instead of requiring the caller to append an LF to the buffer (and
> > potentially allocate memory to do so), the write_message() function
> > learns to append an LF at the end of the file.
>
> As no existing callers need this, it probably is better left out and
> added to the series that actually needs the new feature as a
> preparatory step.
Apart from this patch series being semantically the right place
("prepare-sequencer"), there is also the following consideration:
The next patch series is already quite long. Taking this current patch
series into account, which started out as a 22-patch series and needed to
bloat by 25% through four subsequent iterations, it is probably not a wise
idea to move this patch to a patch series that already weighs 34 patches.
So I respectfully, and forcefully, disagree,
Dscho
^ 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