* Re: Specifying revisions in the future
From: Philip Oakley @ 2012-02-05 22:58 UTC (permalink / raw)
To: Jakub Narebski, Andreas Schwab; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <201202052324.59941.jnareb@gmail.com>
From: "Jakub Narebski" <jnareb@gmail.com>
To: "Andreas Schwab" <schwab@linux-m68k.org>
Cc: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>; <jpaugh@gmx.us>;
<git@vger.kernel.org>
Sent: Sunday, February 05, 2012 10:24 PM
> On Sun, 5 Feb 2012, Andreas Schwab wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>>>
>>>>> "the successor of <commit>", OTOH, is not well defined, since there
>>>>> can
>>>>> be several successors, and one can't order them reliably (you can't
>>>>> really know the set of successors, because they can exist in different
>>>>> repositories).
>>>>
>>>> Yet it would be nice to have a concise notation for "the nth successor
>>>> of <commit> towards <commit>" (using --first-parent ordering when
>>>> ambiguous).
>>>
>>> First, "the nth successor"... from which refs?
>>
>> From the first given commit towards the other given commit (the latter
>> defaulting to HEAD).
>
> That helps some, but not all situations, see below.
>
>> > Second, `--first-parent' won't help here. Take for example the
>> > following situation:
>> >
>> > ---X<---*<---.<---A
>> > \
>> > \--.<---B
>> >
>> > X+3 is A or B?
>>
>> If "towards A" then it is A, if "towards B", it is B. In other words,
>> to get the "nth successor of C1 towards C2" take the leftmost possible
>> parent when walking from C2 to C1, then walk back n commits along this
>> path. This way you should have an unambigous definition.
>
> Nope, still ambiguous:
>
>
>
> ---X<---*<---.<---A<---.<---M<---
> \ /
> \--.<---B<------/
>
> Is X+3 A or B? Though '--first-parent + towards N' is I think
> unambiguous.
> --
Is there also a rule missing for X+2, viewed from D, in this example
X<---Y<---Z<---
\ \
A<----B<----C<----D
as to which order the first parent rule should _not_ be applied when D's
first parent chain doesn't reach X (it reaches A).
Using 'oldest' first for alternate parent testing would make X+2 = B, whilst
'newest' first would make X+2=Z. I have used the chain order for
'newest/oldest', rather than commit date.
(I'm sure that there already exists a natural rule in the dag walk order).
Philip
^ permalink raw reply
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 22:58 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Matthieu Moy, jpaugh, git
In-Reply-To: <201202052324.59941.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Nope, still ambiguous:
>
>
>
> ---X<---*<---.<---A<---.<---M<---
> \ /
> \--.<---B<------/
>
> Is X+3 A or B? Though '--first-parent + towards N' is I think unambiguous.
M
X+3->M is A, if A is the leftmost ancestor of M.
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
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-05 23:08 UTC (permalink / raw)
To: Philip Oakley; +Cc: Jakub Narebski, Matthieu Moy, jpaugh, git
In-Reply-To: <178AA8FDB02246D9AA9416C0D54E51A8@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> Is there also a rule missing for X+2, viewed from D, in this example
>
>X<---Y<---Z<---
> \ \
>A<----B<----C<----D
This is difficult to interpret since it has some extra indent, let's
assume that Z is the second parent of D and Y the second parent of B.
> as to which order the first parent rule should _not_ be applied when D's
> first parent chain doesn't reach X (it reaches A).
> Using 'oldest' first for alternate parent testing would make X+2 = B,
> whilst 'newest' first would make X+2=Z. I have used the chain order for
> newest/oldest', rather than commit date.
The rule should be to follow the leftmost parent as far as possible.
That means that X+2->D is B.
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
* Re: [RFC/PATCH] tag: add --points-at list option
From: Junio C Hamano @ 2012-02-05 23:31 UTC (permalink / raw)
To: Tom Grennan; +Cc: git, krh, jasampler
In-Reply-To: <1328480887-27463-1-git-send-email-tmgrennan@gmail.com>
Tom Grennan <tmgrennan@gmail.com> writes:
> @@ -105,16 +107,28 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> return 0;
> }
>
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf || !size)
> + return 0;
> +
> + if (filter->points_at) {
> + unsigned char tagged_sha1[20];
> + if (memcmp("object ", buf, 7) \
> + || buf[47] != '\n' \
> + || get_sha1_hex(buf + 7, tagged_sha1) \
> + || memcmp(filter->points_at, tagged_sha1, 20)) {
Do we need these backslashes at the end of these lines?
> @@ -143,16 +157,20 @@ static int show_reference(const char *refname, const unsigned char *sha1,
> }
>
> static int list_tags(const char **patterns, int lines,
> - struct commit_list *with_commit)
> + struct commit_list *with_commit,
> + unsigned char *points_at)
> {
It strikes me somewhat odd that you can give a list of commits to filter
when using "--contains" (e.g. "--contains v1.7.9 --contains 1.7.8.4"), but
you can only ask for a single object with "--points-at" from the UI point
of view.
> @@ -375,12 +393,28 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
> return check_refname_format(sb->buf, 0);
> }
>
> +int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
> +{
> + unsigned char *sha1;
> +
> + if (!arg)
> + return -1;
> + sha1 = xmalloc(20);
> + if (get_sha1(arg, sha1)) {
> + free(sha1);
> + return error("malformed object name %s", arg);
> + }
> + *(unsigned char **)opt->value = sha1;
> + return 0;
> +}
We are ignoring earlier --points-at argument without telling the user that
we do not support more than one.
Would it become too much unnecessary addition of new code if you supported
multiple --points-at on the command line for the sake of consistency?
> @@ -417,6 +451,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> PARSE_OPT_LASTARG_DEFAULT,
> parse_opt_with_commit, (intptr_t)"HEAD",
> },
> + {
> + OPTION_CALLBACK, 0, "points-at", &points_at, "object",
> + "print only annotated|signed tags of the object",
> + PARSE_OPT_LASTARG_DEFAULT,
> + parse_opt_points_at, (intptr_t)"HEAD",
> + },
I wonder if defaulting to HEAD even makes sense for --points-at. When you
are chasing a bug and checked out an old version that originally had
problem, "git tag --contains" that defaults to HEAD does have a value. It
tells us what releases are potentially contaminated with the buggy commit.
But does a similar use case support points-at that defaults to HEAD?
Other than that, thanks for a pleasant read.
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-05 23:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vipjlezas.fsf@alter.siamese.dyndns.org>
On Sun, Feb 05, 2012 at 01:38:19PM -0800, Junio C Hamano wrote:
> > The map_user() API takes an email address that is terminated by either NUL
> > or '>' to allow the caller to learn the corresponding up-to-date email
> > address that is NUL terminated, while indicating with its return value
> > that if the caller even needs to replace what it already has. But the
> > function does not properly terminate email when it only touched the name
> > part. And I think that is the real bug.
>
> And the gist of the patch to fix the bug would look like this two liner.
> In the real fix, "p" should be renamed to "end_of_email" or something
> descriptive like that.
Exactly; this is much better.
We could also go as far as saying that map_user would _always_ terminate
in this way (i.e., the caller gets a munged result, whether we found
anything or not). Then internally, map_user could be simplified to stop
worrying about making a temporary copy in mailbuf. And callers could
simply call map_user without worrying about branching on whether it
found anything or not.
But maybe it is not worth that level of refactoring. From my reading,
your patch fixes the problem just fine.
-Peff
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-05 23:50 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s0xmvjxsE6AYrA5qTZuDfWq8vPDPOo69hiRS+xWbsbS7g@mail.gmail.com>
On Sun, Feb 05, 2012 at 11:11:20PM +0200, Felipe Contreras wrote:
> > Because of the calling convention of map_user, the buffer with the input
> > must also be writable (since it holds the result). So there should be no
> > loss of efficiency to convert the ">" into a "\0" (and in fact, the
> > simplest fix is probably to just have map_user "tie off" any ">" it
> > detects).
>
> Yes, but then the caller (git blame) would need to _always_ do that
> conversion before (">" -> "\0"), and after ("\0" -> ">"), as opposed
> to now, that it does the conversion only when map_user succeeds (or
> checks if it has to do it).
Yes, I'm talking about changing the calling and return conventions of
map_user. I think the efficiency change is negligible, though, as we
are talking about character assignments (and in fact, it would probably
end up more efficient, as we could eliminate some copying inside
map_user). But Junio's patch is simple, and fixes the problem without
creating any complexity for the callers. So I think it's a good fix.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] tag: add --points-at list option
From: Jeff King @ 2012-02-06 0:04 UTC (permalink / raw)
To: Tom Grennan; +Cc: git, gitster, krh, jasampler
In-Reply-To: <1328480887-27463-1-git-send-email-tmgrennan@gmail.com>
On Sun, Feb 05, 2012 at 02:28:07PM -0800, Tom Grennan wrote:
> This filters the list for annotated|signed tags of the given object.
> Example,
>
> john$ git tag -s v1.0-john v1.0
> john$ git tag -l --points-at v1.0
> v1.0-john
I really like this approach. One big question, and a few small comments:
> +--points-at <object>::
> + Only list annotated or signed tags of the given object.
> +
It is unclear to me from this documentation if we will only peel a
single level, or if we will peel indefinitely. E.g., what will this
show:
$ git tag one v1.0
$ git tag two one
$ git tag --points-at=v1.0
It will clearly show "one", but will it also show "two" (from reading
the code, I think the answer is "no")? If not, should it?
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf || !size)
> + return 0;
Before your patch, a tag whose sha1 could not be read would get its name
printed, and then we would later return without printing anything more.
Now it won't get even the first bit printed.
However, I'm not sure the old behavior wasn't buggy; it would print part
of the line, but never actually print the newline.
> + if (filter->points_at) {
> + unsigned char tagged_sha1[20];
> + if (memcmp("object ", buf, 7) \
> + || buf[47] != '\n' \
> + || get_sha1_hex(buf + 7, tagged_sha1) \
> + || memcmp(filter->points_at, tagged_sha1, 20)) {
> + free(buf);
> + return 0;
> + }
> + }
Hmm, I would have expected to use parse_tag_buffer instead of doing it
by hand. This is probably a tiny bit more efficient, but I wonder if the
code complexity is worth it.
> static int list_tags(const char **patterns, int lines,
> - struct commit_list *with_commit)
> + struct commit_list *with_commit,
> + unsigned char *points_at)
Like Junio, I was surprised this did not allow a list.
-Peff
^ permalink raw reply
* merge: do not create a signed tag merge under --ff-only option
From: Junio C Hamano @ 2012-02-06 0:22 UTC (permalink / raw)
To: git; +Cc: Bart Trojanowski
In-Reply-To: <7vd39vlbgj.fsf@alter.siamese.dyndns.org>
Starting at release v1.7.9, if you ask to merge a signed tag, "git merge"
always creates a merge commit, even when the tag points at a commit that
happens to be a descendant of your current commit.
Unfortunately, this interacts rather badly for people who use --ff-only to
make sure that their branch is free of local developments. It used to be
possible to say:
$ git checkout -b frotz v1.7.9~30
$ git merge --ff-only v1.7.9
and expect that the resulting tip of frotz branch matches v1.7.9^0 (aka
the commit tagged as v1.7.9), but this fails with the updated Git with:
fatal: Not possible to fast-forward, aborting.
because a merge that merges v1.7.9 tag to v1.7.9~30 cannot be created by
fast forwarding.
We could teach users that now they have to do
$ git merge --ff-only v1.7.9^0
but it is far more pleasant for users if we DWIMmed this ourselves.
When an integrator pulls in a topic from a lieutenant via a signed tag,
even when the work done by the lieutenant happens to fast-forward, the
integrator wants to have a merge record, so the integrator will not be
asking for --ff-only when running "git pull" in such a case. Therefore,
this change should not regress the support for the use case v1.7.9 wanted
to add.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
We could solve this in one of two ways. We could tell them to merge
v3.2.3^0 instead. Or we could just go ahead and do that for them
automatically ourselves. I am inclined to say that we should unwrap
the tag given from the command line when --ff-only was given, i.e. we
do the latter.
And it turns out that it is just a single-liner patch.
builtin/merge.c | 3 ++-
t/t7600-merge.sh | 13 +++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 3a45172..b4fbc60 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1283,7 +1283,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
sha1_to_hex(commit->object.sha1));
setenv(buf.buf, argv[i], 1);
strbuf_reset(&buf);
- if (merge_remote_util(commit) &&
+ if (!fast_forward_only &&
+ merge_remote_util(commit) &&
merge_remote_util(commit)->obj &&
merge_remote_util(commit)->obj->type == OBJ_TAG) {
option_edit = 1;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5d8c428..a598dfa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -27,6 +27,7 @@ Testing basic merge operations/option parsing.
'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
printf '%s\n' 1 2 3 4 5 6 7 8 9 >file
printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >file.1
@@ -670,4 +671,16 @@ test_expect_success 'merge --no-ff --edit' '
test_cmp actual expected
'
+test_expect_success GPG 'merge --ff-only tag' '
+ git reset --hard c0 &&
+ git commit --allow-empty -m "A newer commit" &&
+ git tag -s -m "A newer commit" signed &&
+ git reset --hard c0 &&
+
+ git merge --ff-only signed &&
+ git rev-parse signed^0 >expect &&
+ git rev-parse HEAD >actual &&
+ test_cmp actual expect
+'
+
test_done
^ permalink raw reply related
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06 0:39 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120205234750.GA28735@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We could also go as far as saying that map_user would _always_ terminate
> in this way (i.e., the caller gets a munged result, whether we found
> anything or not). Then internally, map_user could be simplified to stop
> worrying about making a temporary copy in mailbuf. And callers could
> simply call map_user without worrying about branching on whether it
> found anything or not.
I thought about it, but such a change needs to audit all the call sites
that assumes the promise original map_user() used to make before it was
broken. If we return 0 to the caller, the caller does not have to worry
about map_user() munging the buffer it lent to it.
It might be a worthwhile thing to do. I dunno; I didn't look into it.
^ permalink raw reply
* [PATCH] Fix build problems related to profile-directed optimization
From: Theodore Ts'o @ 2012-02-06 0:44 UTC (permalink / raw)
To: git; +Cc: Theodore Ts'o, Andi Kleen
In-Reply-To: <7vaa4zpu2r.fsf@alter.siamese.dyndns.org>
There was a number of problems I ran into when trying the
profile-directed optimizations added by Andi Kleen in git commit
7ddc2710b9. (This was using gcc 4.4 found on many enterprise
distros.)
1) The -fprofile-generate and -fprofile-use commands are incompatible
with ccache; the code ends up looking in the wrong place for the gcda
files based on the ccache object names.
2) If the makefile notices that CFLAGS are different, it will rebuild
all of the binaries. Hence the recipe originally specified by the
INSTALL file ("make profile-all" followed by "make install") doesn't
work. It will appear to work, but the binaries will end up getting
built with no optimization.
This patch fixes this by using an explicit set of options passed via
the PROFILE variable then using this to directly manipulate CFLAGS and
EXTLIBS.
The developer can run "make PROFILE=BUILD all ; sudo make
PROFILE=BUILD install" automatically run a two-pass build with the
test suite run in between as the sample workload for the purpose of
recording profiling information to do the profile-directed
optimization.
Alternatively, the profiling version of binaries can be built using:
make PROFILE=GEN PROFILE_DIR=/var/cache/profile all
make PROFILE=GEN install
and then after git has been used for a while, the optimized version of
the binary can be built as follows:
make PROFILE=USE PROFILE_DIR=/var/cache/profile all
make PROFILE=USE install
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andi Kleen <ak@linux.intel.com>
---
INSTALL | 17 +++++++++++++----
Makefile | 53 +++++++++++++++++++++++++++++++++++------------------
2 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/INSTALL b/INSTALL
index 6fa83fe..5b7eec1 100644
--- a/INSTALL
+++ b/INSTALL
@@ -28,16 +28,25 @@ set up install paths (via config.mak.autogen), so you can write instead
If you're willing to trade off (much) longer build time for a later
faster git you can also do a profile feedback build with
- $ make profile-all
- # make prefix=... install
+ $ make --prefix=/usr PROFILE=BUILD all
+ # make --prefix=/usr PROFILE=BUILD install
This will run the complete test suite as training workload and then
rebuild git with the generated profile feedback. This results in a git
which is a few percent faster on CPU intensive workloads. This
may be a good tradeoff for distribution packagers.
-Note that the profile feedback build stage currently generates
-a lot of additional compiler warnings.
+Or if you just want to install a profile-optimized version of git into
+your home directory, you could run:
+
+ $ make PROFILE=BUILD install
+
+As a caveat: a profile-optimized build takes a *lot* longer since it
+is the sources have to be built twice, and in order for the profiling
+measurements to work properly, ccache must be disabled and the test
+suite has to be run using only a single CPU. In addition, the profile
+feedback build stage currently generates a lot of additional compiler
+warnings.
Issues of note:
diff --git a/Makefile b/Makefile
index c457c34..8cea247 100644
--- a/Makefile
+++ b/Makefile
@@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7
export ASCIIDOC7
endif
+### profile feedback build
+#
+
+# Can adjust this to be a global directory if you want to do extended
+# data gathering
+PROFILE_DIR := $(CURDIR)
+
+ifeq "$(PROFILE)" "GEN"
+ CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
+ EXTLIBS += -lgcov
+ export CCACHE_DISABLE=t
+ V=1
+else ifneq "$PROFILE" ""
+ CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
+ export CCACHE_DISABLE=t
+ V=1
+endif
+
# Shell quote (do not use $(call) to accommodate ancient setups);
SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
@@ -1828,7 +1846,17 @@ export DIFF TAR INSTALL DESTDIR SHELL_PATH
SHELL = $(SHELL_PATH)
-all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test
+
+ifeq "$(PROFILE)" "BUILD"
+ifeq ($(filter all,$(MAKECMDGOALS)),all)
+all:: profile-clean
+ $(MAKE) PROFILE=GEN all
+ $(MAKE) PROFILE=GEN -j1 test
+endif
+endif
+
+all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
ifneq (,$X)
$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
endif
@@ -2557,7 +2585,11 @@ distclean: clean
$(RM) configure
$(RM) po/git.pot
-clean:
+profile-clean:
+ $(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+ $(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+
+clean: profile-clean
$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \
builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
@@ -2587,7 +2619,7 @@ ifndef NO_TCLTK
endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS
-.PHONY: all install clean strip
+.PHONY: all install profile-clean clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
.PHONY: FORCE cscope
@@ -2697,18 +2729,3 @@ cover_db: coverage-report
cover_db_html: cover_db
cover -report html -outputdir cover_db_html cover_db
-### profile feedback build
-#
-.PHONY: profile-all profile-clean
-
-PROFILE_GEN_CFLAGS := $(CFLAGS) -fprofile-generate -DNO_NORETURN=1
-PROFILE_USE_CFLAGS := $(CFLAGS) -fprofile-use -fprofile-correction -DNO_NORETURN=1
-
-profile-clean:
- $(RM) $(addsuffix *.gcda,$(object_dirs))
- $(RM) $(addsuffix *.gcno,$(object_dirs))
-
-profile-all: profile-clean
- $(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all
- $(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test
- $(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all
--
1.7.9.107.g8e04a
^ permalink raw reply related
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Steven Michalske @ 2012-02-06 0:57 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201202042045.54114.jnareb@gmail.com>
See inlined responses below.
On Feb 4, 2012, at 11:45 AM, Jakub Narebski wrote:
> So people would like for git to warn them about rewriting history before
> they attempt a push and it turns out to not fast-forward.
>
I like this idea and I encounter this issue with my co-workers new to git.
It scares them thinking they broke the repository.
> In Mercurial 2.1 there are three available phases: 'public' for
> published commits, 'draft' for local un-published commits and
> 'secret' for local un-published commits which are not meant to
> be published.
>
> The phase of a changeset is always equal to or higher than the phase
> of it's descendants, according to the following order:
>
> public < draft < secret
Let's not limit ourselves to just three levels. They are a great start but I propose the following.
published - The commits that are on a public repository that if are rewritten will invoke uprisings.
general rule here would be to revert or patch, no rewrites.
based - The commits that the core developers have work based upon. (not just the commits in their repo.)
general rule is notify your fellow developers before a rewrite.
shared - The commits that are known to your fellow core developers.
These commits are known, but have not had work based off of them. Minimal risk to rewrite.
local - The commits that are local only, no one else has a copy.
Commits your willing to share, but have not been yet shared, either from actions of you, or a fetch from others.
restricted or private - The commits that you do not want shared.
Manually added, think of a branch tip marked as restricted automatically promotes commits to the branch as restricted.
Maybe make these like nice levels, but as two components, publicity 0-100 and rewritability 0-100
Published is publicity 100 and rewritability 0
Restricted is publicity 0 and rewritability 100
Based publicity 75 and rewritability 25
Shared publicity 50 and rewritability 50
Local publicity 25 and rewritability 75
Restricted publicity 0 and rewritability 100
Other option are flags stating if the commit is published, based, shared, or restricted.
You could have a published and based commit that is more opposed to rewrite than a public commit.
Call security on a published restricted commit ;-)
Commits are by default local.
Commits are published when they are pushed or fetched and merged to a publishing branch of a repository.
On fetch/merge a post merge hook should send back a note to the remote repository that the commits were published.
Restricted commits/branches/tags should not be made public, error out and require clearing of the attribute or a --force-restricted option that automatically removes the restricted attribute. They are at least promoted to shared, if not published.
Based is only used in situations where you have developers sharing amongst their repositories, and you want a rule that is less restrictive than no rewrites.
Shared is what we have now when a commit is in a remote repository without the no rewrite options. e.g. receive.denyNonFastForwards.
As it stands now we can infer local and shared, we need metadata to know when a commit is made based, published, or restricted.
Using the nomenclature from Mercurial
> public < draft < secret
public -> publicity 100, rewritability 0
draft -> publicity ?, rewritability 50
secret -> publicity 0, rewritability 100
Steve
^ permalink raw reply
* [PATCH] branch --edit-description: protect against mistyped branch name
From: Junio C Hamano @ 2012-02-06 1:26 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty, Jeff King
In-Reply-To: <20120130214842.GA16149@sigill.intra.peff.net>
It is very easy to mistype the branch name when editing its description,
e.g.
$ git checkout -b my-topic master
: work work work
: now we are at a good point to switch working something else
$ git checkout master
: ah, let's write it down before we forget what we were doing
$ git branch --edit-description my-tpoic
The command does not notice that branch 'my-tpoic' does not exist. It is
not lost (it becomes description of an unborn my-tpoic branch), but is not
very useful. So detect such a case and error out to reduce the grief
factor from this common mistake.
This incidentally also errors out --edit-description when the HEAD points
at an unborn branch (immediately after "init", or "checkout --orphan"),
because at that point, you do not even have any commit that is part of
your history and there is no point in describing how this particular
branch is different from the branch it forked off of, which is the useful
bit of information the branch description is designed to capture.
We may want to special case the unborn case later, but that is outside the
scope of this patch to prevent more common mistakes before 1.7.9 series
gains too much widespread use.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Jeff King <peff@peff.net> writes:
> IOW, the problem with the current code is that it allows typos and other
> arbitrary bogus names to be silently described, even though doing so is
> probably an error...
builtin/branch.c | 15 +++++++++++++++
t/t3200-branch.sh | 41 +++++++++++++++++++++++++++++++++++++----
2 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 7095718..0c1784f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -768,6 +768,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
with_commit, argv);
else if (edit_description) {
const char *branch_name;
+ struct strbuf branch_ref = STRBUF_INIT;
+
if (detached)
die("Cannot give description to detached HEAD");
if (!argc)
@@ -776,6 +778,19 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
branch_name = argv[0];
else
usage_with_options(builtin_branch_usage, options);
+
+ strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
+ if (!ref_exists(branch_ref.buf)) {
+ strbuf_reset(&branch_ref);
+
+ if (!argc)
+ return error("No commit on branch '%s' yet.",
+ branch_name);
+ else
+ return error("No such branch '%s'.", branch_name);
+ }
+ strbuf_reset(&branch_ref);
+
if (edit_branch_description(branch_name))
return 1;
} else if (rename) {
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ea82424..dd1aceb 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -3,11 +3,8 @@
# Copyright (c) 2005 Amos Waterland
#
-test_description='git branch --foo should not create bogus branch
+test_description='git branch assorted tests'
-This test runs git branch --help and checks that the argument is properly
-handled. Specifically, that a bogus branch is not created.
-'
. ./test-lib.sh
test_expect_success \
@@ -620,4 +617,40 @@ test_expect_success 'use set-upstream on the current branch' '
'
+test_expect_success 'use --edit-description' '
+ write_script editor <<-\EOF &&
+ echo "New contents" >"$1"
+ EOF
+ EDITOR=./editor git branch --edit-description &&
+ write_script editor <<-\EOF &&
+ git stripspace -s <"$1" >"EDITOR_OUTPUT"
+ EOF
+ EDITOR=./editor git branch --edit-description &&
+ echo "New contents" >expect &&
+ test_cmp EDITOR_OUTPUT expect
+'
+
+test_expect_success 'detect typo in branch name when using --edit-description' '
+ write_script editor <<-\EOF &&
+ echo "New contents" >"$1"
+ EOF
+ (
+ EDITOR=./editor &&
+ export EDITOR &&
+ test_must_fail git branch --edit-description no-such-branch
+ )
+'
+
+test_expect_success 'refuse --edit-description on unborn branch for now' '
+ write_script editor <<-\EOF &&
+ echo "New contents" >"$1"
+ EOF
+ git checkout --orphan unborn &&
+ (
+ EDITOR=./editor &&
+ export EDITOR &&
+ test_must_fail git branch --edit-description
+ )
+'
+
test_done
--
1.7.9.172.ge26ae
^ permalink raw reply related
* Re: [PATCH] branch --edit-description: protect against mistyped branch name
From: Junio C Hamano @ 2012-02-06 1:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty, Jeff King
In-Reply-To: <7vaa4wda60.fsf_-_@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> + if (!ref_exists(branch_ref.buf)) {
> + strbuf_reset(&branch_ref);
> +
> + if (!argc)
> + return error("No commit on branch '%s' yet.",
> + branch_name);
> + else
> + return error("No such branch '%s'.", branch_name);
> + }
> + strbuf_reset(&branch_ref);
> +
Of course these should be strbuf_release().
^ permalink raw reply
* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Felipe Contreras @ 2012-02-06 1:27 UTC (permalink / raw)
To: Thomas Rast
Cc: git, Brandon Casey, Uwe Kleine-König, Brian Gernhardt,
Robin H. Johnson, Ævar Arnfjörð
In-Reply-To: <877h01rlsr.fsf@thomas.inf.ethz.ch>
On Sun, Feb 5, 2012 at 11:51 PM, Thomas Rast <trast@inf.ethz.ch> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> 2012/2/5 Thomas Rast <trast@inf.ethz.ch>:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>>
>>>> 'Foo Bar <foo@bar.com>' -> 'Foo Bar <foo@bar.com>'
>>>> '"Foo Bar" <foo@bar.com>' -> '"Foo Bar" <foo@bar.com>'
>>>> 'foo@bar.com' -> 'foo@bar.com'
>>>> '<foo@bar.com>' -> 'foo@bar.com'
>>>> 'Foo Bar' -> 'Foo Bar'
>>>
>>> Am I the only one who stared at this for ten seconds, only to then
>>> realize that there is no sanitizing whatsoever going on here?
>>
>> There is: '<foo@bar.com>' -> 'foo@bar.com'
>
> Indeed.
>
> I still feel cheated as a reader though, you showed me four examples of
> no change but let me figure that on my own.
>
>>>> 'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
>>>> '"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
>>>> '<foo@bar.com>>' -> 'foo@bar.com'
>>>
>>> All of these are the same underlying issue. Does your patch fix any
>>> other malformed addresses, or just this particular type?
>>
>> See above.
>
> Ok, I see I am falling into the same communication trap as Jonathan, so
> let's be more explicit.
>
> Your commit message first tells me you are going to sanitize something,
> but starts out with examples of leaving the string unchanged. Then it
> continues with only the '>>' examples.
Which is why I added a paragraph to explain them. What is unclear about?
---
According to commit 155197e[1], the "prhase" should not be empty, so
if it is, remove the
<>. Extra characters after the first ">" are ignored.
---
> Today, and being someone who on average reads about half the mail that
> comes through here, I know that this relates to the blame -e '>>' bug.
> So today, I am wondering from the commit message why you narrowly focus
> on that bug. But you don't! It's just that the commit message
> insinuates it.
The summary explains the purpose of the patch "add extra safety in
address sanitation" (should fix those typos though).
> In a year, your reader (and bear in mind that this may very well be
> yourself, at least if your memory is as good as mine) will wonder what
> was so damn special about that '>>' string that it needs a specific fix
> to send-email.
It doesn't matter, could be "<foo@bar.com> err blop", or any number of
other malformed strings.
> I see that you wrote in another thread:
>
>> I have to write a peer-reviewed essay with an introduction for the
>> people that are not familiar with the code in each of the patches
>
> I'm not sure you meant it that literally, but the whole *point* is that
> the message is for people who are not familiar with the code. After
> all, if I knew that your code did the right thing in the right way, I
> would not be bothering with reading the message. Today, I would just
> send an Acked-by instead. In a year, I'd scroll down for another
> potential culprit for the bug I'm hunting.
You are assuming too much. In this case, the code is clear and doesn't
need explaining. I am talking about other cases which in my mind are
akin to explaining what is $recipient, $recipient_name, and what does
sanitize_address does, and why the if case for is_rfc2047_quoted is
modified. IMO that's overkill.
If you have some suggestion about how to improve the commit message, I
would be glad to listen to them, as in this case, I do believe the
changes merit some clear explanation. Not all patches do, though.
> What's especially striking me about your proposed messages of late: they
> leave me with more open questions than I started with. I tried to show
> this above. I'm not sure whether other contributors are better at
> answering questions, or just better at not touching any topics that
> might raise them.
Again, what is not clear about:
---
Basically, we try to check that the address is in the form of
"Name <email>", and if not, assume it's "email". According to commit
155197e[1], the "prhase" should not be empty, so if it is, remove the
<>. Extra characters after the first ">" are ignored.
---
To me that explains what the patch is trying to do: "add extra safety
in address sanitation".
Anyway, it seems people don't care if 'git send-email' attempts to
send random garbage regardless, so I'm not going to pursue this patch.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 0/3] On compresing large index
From: Nguyen Thai Ngoc Duy @ 2012-02-06 1:35 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Joshua Redstone
In-Reply-To: <87ehu9ug9i.fsf@thomas.inf.ethz.ch>
2012/2/6 Thomas Rast <trast@inf.ethz.ch>:
>> We need to figure out what git uses 4s user time for.
>
> When I worked on the cache-tree stuff, my observation (based on
> profiling, so I had actual data :-) was that computing SHA1s absolutely
> dominates everything in such operations. It does that when writing the
> index to write the trailing checksum, and also when loading it to verify
> that the index is valid.
You're right. This is on another machine but with same index (2M
files), without SHA1 checksum:
$ time ~/w/git/git ls-files --stage|head > /dev/null
real 0m1.533s
user 0m1.228s
sys 0m0.306s
and with SHA-1 checksum:
$ time git ls-files --stage|head > /dev/null
real 0m7.525s
user 0m7.257s
sys 0m0.268s
I guess we could fall back to cheaper digests for such a large index.
Still more than one second for doing nothing but reading index is too
slow to me.
> ls-files shouldn't be so slow though. A quick run with callgrind in a
> linux-2.6.git tells me it spends about 45% of its time on SHA1s and a
> whopping 25% in quote_c_style(). I wonder what's so hard about
> quoting...
That's why I put "| head" there, to cut output processing overhead (hopefully).
--
Duy
^ permalink raw reply
* Re: [PATCH] send-email: add extra safetly in address sanitazion
From: Junio C Hamano @ 2012-02-06 1:48 UTC (permalink / raw)
To: Felipe Contreras
Cc: Thomas Rast, git, Brandon Casey, Uwe Kleine-König,
Brian Gernhardt, Robin H. Johnson, Ævar Arnfjörð
In-Reply-To: <CAMP44s30VmJasMLJxs-JFwksvPEPpG1LB3Gr_pA2+hpE1AnwXg@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Anyway, it seems people don't care if 'git send-email' attempts to
> send random garbage regardless, so I'm not going to pursue this patch.
I actually think people _do_ care, and that is the _only_ reason you are
getting review comments.
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06 2:06 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty, Jeff King
In-Reply-To: <20120130215043.GB16149@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Is it really worth warning? After all, by definition you are not leaving
> any commits or useful work behind.
I actually do not know if this change itself is worth doing, but if we
were to do this, then I think the user benefits from the warning.
The patch is made on maint-1.7.6 track for no good reason, so it may have
some merge conflicts around "resolve_ref()" vs "resolve_refdup()" if we
were to apply it on a more modern codebase, but the resolution should be
trivial.
---
Subject: [PATCH] git checkout -b: allow switching out of an unborn branch
Running "git checkout -b another" immediately after "git init" when you do
not even have a commit on 'master' is forbidden, with a readable message:
$ git checkout -b another
fatal: You are on a branch yet to be born
It is readable but not easily understandable unless the user knows what
"yet to be born" really means.
So let's try allowing it and see what happens. I strongly suspect that
this may just shift the confusion one step further without adding much
value to the resulting system, because the next question that would come
to somebody who does not understand what "yet to be born" is is "why don't
I see 'master' in the output from 'git branch' command?", and the new
warning may not be descriptive enough to explain what the user is doing.
The early part of switch_branches() that computes old is probably be
better moved to the caller cmd_checkout() and used in the new code that
detects the "unborn" case, and passed as to switch_branches() as the third
parameter. Such improvements and tests are left as an exercise for the
interested and motivated, as usual ;-)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/checkout.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c20dae..5894f40 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -916,6 +916,19 @@ static int parse_branchname_arg(int argc, const char **argv,
return argcount;
}
+static int switch_unborn_to_new_branch(struct checkout_opts *opts, const char *old_ref)
+{
+ int status;
+ struct strbuf branch_ref = STRBUF_INIT;
+
+ strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
+ warning(_("Leaving the unborn branch '%s' behind..."),
+ skip_prefix(old_ref, "refs/heads/"));
+ status = create_symref("HEAD", branch_ref.buf, "checkout -b");
+ strbuf_release(&branch_ref);
+ return status;
+}
+
int cmd_checkout(int argc, const char **argv, const char *prefix)
{
struct checkout_opts opts;
@@ -1089,5 +1102,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
if (opts.writeout_stage)
die(_("--ours/--theirs is incompatible with switching branches."));
+ if (!new.commit) {
+ unsigned char rev[20];
+ int flag, status;
+ const char *old_ref = resolve_ref("HEAD", rev, 0, &flag);
+
+ if ((flag & REF_ISSYMREF) && is_null_sha1(rev)) {
+ status = switch_unborn_to_new_branch(&opts, old_ref);
+ free((char *)old_ref);
+ return status;
+ }
+ }
return switch_branches(&opts, &new);
}
--
1.7.9.172.ge26ae
^ permalink raw reply related
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06 2:08 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty, Jeff King
In-Reply-To: <7vobtcbtqa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The patch is made on maint-1.7.6 track for no good reason, so it may have
> some merge conflicts around "resolve_ref()" vs "resolve_refdup()" if we
> were to apply it on a more modern codebase, but the resolution should be
> trivial.
The resolution should look like this, just in case.
diff --cc builtin/checkout.c
index 5bf96ba,5894f40..41b9b34
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@@ -1079,5 -1102,16 +1092,16 @@@ int cmd_checkout(int argc, const char *
if (opts.writeout_stage)
die(_("--ours/--theirs is incompatible with switching branches."));
+ if (!new.commit) {
+ unsigned char rev[20];
+ int flag, status;
- const char *old_ref = resolve_ref("HEAD", rev, 0, &flag);
++ char *old_ref = resolve_refdup("HEAD", rev, 0, &flag);
+
+ if ((flag & REF_ISSYMREF) && is_null_sha1(rev)) {
+ status = switch_unborn_to_new_branch(&opts, old_ref);
- free((char *)old_ref);
++ free(old_ref);
+ return status;
+ }
+ }
return switch_branches(&opts, &new);
}
^ permalink raw reply
* What's cooking in git.git (Feb 2012, #02; Sun, 5)
From: Junio C Hamano @ 2012-02-06 2:44 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in
'next'.
Compared to the recent activity level on discussions of new features on
the list, some people may be wondering if the rate of advancement of the
'master' and 'next' branches is getting throttled.
That is because it is.
Now the obviously good bits that have been cooking during the feature
freeze are pushed out to 'master', I'd want to make sure we can have a
timely release of v1.7.9.1 so that people can start benefiting from the
features and fixes introduced in v1.7.9 more smoothly and sooner, and that
is where my focus lies at this moment. I've been picking up new topics and
adding them to 'pu' only "as time and attention permit" basis, and this
mode of operation probably will continue throughout the second week of the
post v1.7.9 cycle (cf. http://tinyurl.com/gitcal).
Here are the repositories that have my integration branches:
With maint, master, next, pu, todo:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/git
With only maint and master:
git://git.sourceforge.jp/gitroot/git-core/git.git
git://git-core.git.sourceforge.net/gitroot/git-core/git-core
With all the topics and integration branches:
https://github.com/gitster/git
The preformatted documentation in HTML and man format are found in:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://code.google.com/p/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
--------------------------------------------------
[New Topics]
* bw/inet-pton-ntop-compat (2012-02-05) 1 commit
- Drop system includes from inet_pton/inet_ntop compatibility wrappers
The inclusion order of header files bites Solaris again and this fixes it.
Will merge to 'next'.
* jc/branch-desc-typoavoidance (2012-02-05) 2 commits
- branch --edit-description: protect against mistyped branch name
+ tests: add write_script helper function
(this branch is tangled with jk/tests-write-script.)
Typo in "git branch --edit-description my-tpoic" was not diagnosed.
Will merge to 'next'.
* jc/checkout-out-of-unborn (2012-02-05) 1 commit
- git checkout -b: allow switching out of an unborn branch
I am fairly negative on this one, as I think it is just shifting the
problem around.
* jc/maint-mailmap-output (2012-02-05) 1 commit
- mailmap: do not leave '>' in the output when answering "we did something"
map_user() was not rewriting its output correctly, which resulted in the
user visible symptom that "git blame -e" sometimes showed excess '>' at
the end of email addresses.
* jc/merge-ff-only-stronger-than-signed-merge (2012-02-05) 1 commit
- merge: do not create a signed tag merge under --ff-only option
"git merge --ff-only $tag" failed because it cannot record the required
mergetag without creating a merge, but this is so common operation for
branch that is used _only_ to follow the upstream, so it is allowed to
fast-forward without recording the mergetag.
* tt/profile-build-fix (2012-02-05) 1 commit
- Fix build problems related to profile-directed optimization
--------------------------------------------------
[Stalled]
* jc/advise-push-default (2011-12-18) 1 commit
- push: hint to use push.default=upstream when appropriate
Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?
* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
- fixup! 15eaaf4
- git-svn, perl/Git.pm: extend Git::prompt helper for querying users
- perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS
The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.
* nd/commit-ignore-i-t-a (2012-01-16) 2 commits
- commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees
- cache-tree: update API to take abitrary flags
May want to consider this as fixing an earlier UI mistake, and not as a
feature that devides the userbase.
* jc/split-blob (2012-01-24) 6 commits
- chunked-object: streaming checkout
- chunked-object: fallback checkout codepaths
- bulk-checkin: support chunked-object encoding
- bulk-checkin: allow the same data to be multiply hashed
- new representation types in the packstream
- varint-in-pack: refactor varint encoding/decoding
Not ready.
I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.
Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.
--------------------------------------------------
[Cooking]
* nd/diffstat-gramnum (2012-02-03) 1 commit
(merged to 'next' on 2012-02-05 at 7335ecc)
+ Use correct grammar in diffstat summary line
The commands in the "git diff" family and "git apply --stat" that count
the number of files changed and the number of lines inserted/deleted have
been updated to match the output from "diffstat". This also opens the
door to i18n this line.
* jx/i18n-more-marking (2012-02-01) 2 commits
(merged to 'next' on 2012-02-05 at 44e8cf6)
+ i18n: format_tracking_info "Your branch is behind" message
+ i18n: git-commit whence_s "merge/cherry-pick" message
Marks a few more messages we forgot to mark for i18n.
* jk/grep-binary-attribute (2012-02-02) 9 commits
(merged to 'next' on 2012-02-05 at 9dffa7e)
+ grep: pre-load userdiff drivers when threaded
+ grep: load file data after checking binary-ness
+ grep: respect diff attributes for binary-ness
+ grep: cache userdiff_driver in grep_source
+ grep: drop grep_buffer's "name" parameter
+ convert git-grep to use grep_source interface
+ grep: refactor the concept of "grep source" into an object
+ grep: move sha1-reading mutex into low-level code
+ grep: make locking flag global
Fixes a longstanding bug that there was no way to tell "git grep" that a
path may look like text but it is not, which "git diff" can do using the
attributes system. Now "git grep" honors the same "binary" (or "-diff")
attribute.
* jc/parse-date-raw (2012-02-03) 2 commits
- parse_date(): '@' prefix forces git-timestamp
- parse_date(): allow ancient git-timestamp
"rebase" and "commit --amend" failed to work on commits with ancient
timestamps near year 1970.
Waiting for comments.
* jk/git-dir-lookup (2012-02-02) 1 commit
(merged to 'next' on 2012-02-05 at 1856d74)
+ standardize and improve lookup rules for external local repos
When you have both .../foo and .../foo.git, "git clone .../foo" did not
favor the former but the latter.
* jk/prompt-fallback-to-tty (2012-02-03) 2 commits
- prompt: fall back to terminal if askpass fails
- prompt: clean up strbuf usage
The code to ask for password did not fall back to the terminal input when
GIT_ASKPASS is set but does not work (e.g. lack of X with GUI askpass
helper).
Will merge to 'next'.
* jk/tests-write-script (2012-02-03) 2 commits
(merged to 'next' on 2012-02-05 at 4264ffa)
+ t0300: use write_script helper
+ tests: add write_script helper function
(this branch is tangled with jc/branch-desc-typoavoidance.)
* jn/gitweb-search-utf-8 (2012-02-03) 1 commit
(merged to 'next' on 2012-02-05 at 055e446)
+ gitweb: Allow UTF-8 encoded CGI query parameters and path_info
Search box in "gitweb" did not accept non-ASCII characters correctly.
* jn/rpm-spec (2012-02-03) 1 commit
(merged to 'next' on 2012-02-05 at dba940b)
+ git.spec: Workaround localized messages not put in any RPM
Fix breakage in v1.7.9 Makefile; rpmbuild notices an unpackaged but
installed *.mo file and fails.
* fc/zsh-completion (2012-02-03) 3 commits
- completion: simplify __gitcomp and __gitcomp_nl implementations
- completion: use ls -1 instead of rolling a loop to do that ourselves
- completion: work around zsh option propagation bug
Fix git subcommand completion for zsh (in contrib/completion).
Will merge to 'next'.
* jc/maint-request-pull-for-tag (2012-01-31) 1 commit
(merged to 'next' on 2012-02-01 at 7649f18)
+ request-pull: explicitly ask tags/$name to be pulled
When asking for a tag to be pulled, "request-pull" shows the name of the
tag prefixed with "tags/"
* nd/find-pack-entry-recent-cache-invalidation (2012-02-01) 2 commits
(merged to 'next' on 2012-02-01 at e26aed0)
+ find_pack_entry(): do not keep packed_git pointer locally
+ sha1_file.c: move the core logic of find_pack_entry() into fill_pack_entry()
* nd/pack-objects-parseopt (2012-02-01) 3 commits
(merged to 'next' on 2012-02-05 at d0dc25d)
+ pack-objects: convert to use parse_options()
+ pack-objects: remove bogus comment
+ pack-objects: do not accept "--index-version=version,"
"pack-objects" learned use parse-options, losing custom command line
parsing code.
* bl/gitweb-project-filter (2012-02-01) 8 commits
(merged to 'next' on 2012-02-01 at 2c96ce7)
+ gitweb: Make project search respect project_filter
+ gitweb: improve usability of projects search form
+ gitweb: place links to parent directories in page header
+ gitweb: show active project_filter in project_list page header
+ gitweb: limit links to alternate forms of project_list to active project_filter
+ gitweb: add project_filter to limit project list to a subdirectory
+ gitweb: prepare git_get_projects_list for use outside 'forks'.
+ gitweb: move hard coded .git suffix out of git_get_projects_list
"gitweb" allows intermediate entries in the directory hierarchy that leads
to a projects to be clicked, which in turn shows the list of projects
inside that directory.
* rt/completion-branch-edit-desc (2012-01-29) 1 commit
(merged to 'next' on 2012-02-01 at 0627ebf)
+ completion: --edit-description option for git-branch
Originally merged to 'next' on 2012-01-31.
Will merge to 'master'.
* jn/svn-fe (2012-02-02) 47 commits
(merged to 'next' on 2012-02-05 at e9d3917)
+ vcs-svn: suppress a -Wtype-limits warning
+ vcs-svn: allow import of > 4GiB files
+ vcs-svn: rename check_overflow arguments for clarity
(merged to 'next' on 2012-02-01 at 9288c95)
+ vcs-svn/svndiff.c: squelch false "unused" warning from gcc
+ Merge branch 'svn-fe' of git://repo.or.cz/git/jrn into jn/svn-fe
+ vcs-svn: reset first_commit_done in fast_export_init
+ Merge branch 'db/text-delta' into svn-fe
+ vcs-svn: do not initialize report_buffer twice
+ Merge branch 'db/text-delta' into svn-fe
+ vcs-svn: avoid hangs from corrupt deltas
+ vcs-svn: guard against overflow when computing preimage length
+ Merge branch 'db/delta-applier' into db/text-delta
+ vcs-svn: implement text-delta handling
+ Merge branch 'db/delta-applier' into db/text-delta
+ Merge branch 'db/delta-applier' into svn-fe
+ vcs-svn: cap number of bytes read from sliding view
+ test-svn-fe: split off "test-svn-fe -d" into a separate function
+ vcs-svn: let deltas use data from preimage
+ vcs-svn: let deltas use data from postimage
+ vcs-svn: verify that deltas consume all inline data
+ vcs-svn: implement copyfrom_data delta instruction
+ vcs-svn: read instructions from deltas
+ vcs-svn: read inline data from deltas
+ vcs-svn: read the preimage when applying deltas
+ vcs-svn: parse svndiff0 window header
+ vcs-svn: skeleton of an svn delta parser
+ vcs-svn: make buffer_read_binary API more convenient
+ vcs-svn: learn to maintain a sliding view of a file
+ Makefile: list one vcs-svn/xdiff object or header per line
+ Merge branch 'db/svn-fe-code-purge' into svn-fe
+ vcs-svn: drop obj_pool
+ vcs-svn: drop treap
+ vcs-svn: drop string_pool
+ vcs-svn: pass paths through to fast-import
+ Merge branch 'db/strbufs-for-metadata' into db/svn-fe-code-purge
+ Merge branch 'db/length-as-hash' (early part) into db/svn-fe-code-purge
+ Merge branch 'db/vcs-svn-incremental' into svn-fe
+ vcs-svn: avoid using ls command twice
+ vcs-svn: use mark from previous import for parent commit
+ vcs-svn: handle filenames with dq correctly
+ vcs-svn: quote paths correctly for ls command
+ vcs-svn: eliminate repo_tree structure
+ vcs-svn: add a comment before each commit
+ vcs-svn: save marks for imported commits
+ vcs-svn: use higher mark numbers for blobs
+ vcs-svn: set up channel to read fast-import cat-blob response
+ Merge commit 'v1.7.5' into svn-fe
Originally merged to 'next' on 2012-01-29.
"vcs-svn"/"svn-fe" learned to read dumps with svn-deltas and support
incremental imports.
Will merge to 'master'.
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-06 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vehu8dcc8.fsf@alter.siamese.dyndns.org>
On Sun, Feb 05, 2012 at 04:39:35PM -0800, Junio C Hamano wrote:
> > We could also go as far as saying that map_user would _always_ terminate
> > in this way (i.e., the caller gets a munged result, whether we found
> > anything or not). Then internally, map_user could be simplified to stop
> > worrying about making a temporary copy in mailbuf. And callers could
> > simply call map_user without worrying about branching on whether it
> > found anything or not.
>
> I thought about it, but such a change needs to audit all the call sites
> that assumes the promise original map_user() used to make before it was
> broken. If we return 0 to the caller, the caller does not have to worry
> about map_user() munging the buffer it lent to it.
>
> It might be a worthwhile thing to do. I dunno; I didn't look into it.
Ugh, yeah. I was thinking about how it would improve this call site, but
I don't want to get into auditing the others. Let's drop it and go with
your patch.
-Peff
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06 3:13 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120206030339.GA29123@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> It might be a worthwhile thing to do. I dunno; I didn't look into it.
>
> Ugh, yeah. I was thinking about how it would improve this call site, but
> I don't want to get into auditing the others.
There aren't that many, though. shortlog has one, pretty has another and
that is about it.
But both seems to care that map_user() is not a function that returns void,
so...
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Junio C Hamano @ 2012-02-06 3:16 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120206030339.GA29123@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Ugh, yeah. I was thinking about how it would improve this call site, but
> I don't want to get into auditing the others. Let's drop it and go with
> your patch.
In any case, here is what I queued for tonight.
-- >8 --
Subject: [PATCH] mailmap: do not leave '>' in the output when answering "we did something"
The callers of map_user() give email and name to it, and expect to get an
up-to-date versions of email and/or name to be used in their output. The
function rewrites the given buffers in place. To optimize the majority of
cases, the function returns 0 when it did not do anything, and it returns
1 when the caller should use the updated contents.
The 'email' input to the function is terminated by '>' or a NUL (whichever
comes first) for historical reasons, but when a rewrite happens, the value
is replaced with the mailbox inside the <> pair. However, it failed to
meet this expectation when it only rewrote the name part without rewriting
the email part, and the email in the input was terminated by '>'.
This causes an extra '>' to appear in the output of "blame -e", because the
caller does send in '>'-terminated email, and when the function returned 1
to tell it that rewriting happened, it appends '>' that is necessary when
the email part was rewritten.
The patch looks bigger than it actually is, because this change makes a
variable that points at the end of the email part in the input 'p' live
much longer than it used to, deserving a more descriptive name.
Noticed and diagnosed by Felipe Contreras and Jeff King.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
mailmap.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index 8c3196c..47aa419 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -190,27 +190,27 @@ void clear_mailmap(struct string_list *map)
int map_user(struct string_list *map,
char *email, int maxlen_email, char *name, int maxlen_name)
{
- char *p;
+ char *end_of_email;
struct string_list_item *item;
struct mailmap_entry *me;
char buf[1024], *mailbuf;
int i;
/* figure out space requirement for email */
- p = strchr(email, '>');
- if (!p) {
+ end_of_email = strchr(email, '>');
+ if (!end_of_email) {
/* email passed in might not be wrapped in <>, but end with a \0 */
- p = memchr(email, '\0', maxlen_email);
- if (!p)
+ end_of_email = memchr(email, '\0', maxlen_email);
+ if (!end_of_email)
return 0;
}
- if (p - email + 1 < sizeof(buf))
+ if (end_of_email - email + 1 < sizeof(buf))
mailbuf = buf;
else
- mailbuf = xmalloc(p - email + 1);
+ mailbuf = xmalloc(end_of_email - email + 1);
/* downcase the email address */
- for (i = 0; i < p - email; i++)
+ for (i = 0; i < end_of_email - email; i++)
mailbuf[i] = tolower(email[i]);
mailbuf[i] = 0;
@@ -236,6 +236,8 @@ int map_user(struct string_list *map,
}
if (maxlen_email && mi->email)
strlcpy(email, mi->email, maxlen_email);
+ else
+ *end_of_email = '\0';
if (maxlen_name && mi->name)
strlcpy(name, mi->name, maxlen_name);
debug_mm("map_user: to '%s' <%s>\n", name, mi->email ? mi->email : "");
--
1.7.9.204.gdf845
^ permalink raw reply related
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-06 4:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vy5sgaby1.fsf@alter.siamese.dyndns.org>
On Sun, Feb 05, 2012 at 07:16:22PM -0800, Junio C Hamano wrote:
> In any case, here is what I queued for tonight.
>
> -- >8 --
> Subject: [PATCH] mailmap: do not leave '>' in the output when answering "we did something"
Looks good to me.
> The callers of map_user() give email and name to it, and expect to get an
> up-to-date versions of email and/or name to be used in their output. The
Minor grammar error.
-Peff
^ permalink raw reply
* Re: [PATCH 1/3] blame: fix email output with mailmap
From: Jeff King @ 2012-02-06 4:09 UTC (permalink / raw)
To: Thomas Rast; +Cc: Felipe Contreras, git
In-Reply-To: <87liohvysi.fsf@thomas.inf.ethz.ch>
On Sun, Feb 05, 2012 at 08:57:17PM +0100, Thomas Rast wrote:
> (Granted, omitting *Peff* doesn't make that much of a difference, since
> for all I know he reads every email that crosses this list. But my
> point still stands.)
It's not true at all. I didn't read this message, for instance.
-Peff
^ permalink raw reply
* Re: [PATCH] Fix build problems related to profile-directed optimization
From: Jeff King @ 2012-02-06 4:18 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: git, Andi Kleen
In-Reply-To: <1328489090-14178-1-git-send-email-tytso@mit.edu>
On Sun, Feb 05, 2012 at 07:44:50PM -0500, Theodore Ts'o wrote:
> diff --git a/INSTALL b/INSTALL
> index 6fa83fe..5b7eec1 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -28,16 +28,25 @@ set up install paths (via config.mak.autogen), so you can write instead
> If you're willing to trade off (much) longer build time for a later
> faster git you can also do a profile feedback build with
>
> - $ make profile-all
> - # make prefix=... install
> + $ make --prefix=/usr PROFILE=BUILD all
> + # make --prefix=/usr PROFILE=BUILD install
Eh? --prefix?
> +As a caveat: a profile-optimized build takes a *lot* longer since it
> +is the sources have to be built twice, and in order for the profiling
s/it is//
> diff --git a/Makefile b/Makefile
> index c457c34..8cea247 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7
> [...]
> +ifeq "$(PROFILE)" "GEN"
> + CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
> + EXTLIBS += -lgcov
> + export CCACHE_DISABLE=t
> + V=1
> +else ifneq "$PROFILE" ""
> + CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
> + export CCACHE_DISABLE=t
> + V=1
> +endif
Did you mean "$(PROFILE)" in the second conditional?
-Peff
^ 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