* [PATCH 1/2] format-patch: document and test --reroll-count
From: Junio C Hamano @ 2013-01-02 22:42 UTC (permalink / raw)
To: git
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This comes on top of the 7 patches that have been cooking on 'pu'
(http://thread.gmane.org/gmane.comp.version-control.git/212036),
and I am planning to squash this to 7/7 that adds --reroll-count
option to the code. This is sent as a separate patch to keep
reviewing easier.
Documentation/git-format-patch.txt | 10 +++++++++-
t/t4014-format-patch.sh | 8 ++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 6d43f56..736d8bf 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -18,7 +18,7 @@ SYNOPSIS
[--start-number <n>] [--numbered-files]
[--in-reply-to=Message-Id] [--suffix=.<sfx>]
[--ignore-if-in-upstream]
- [--subject-prefix=Subject-Prefix]
+ [--subject-prefix=Subject-Prefix] [--reroll-count <n>]
[--to=<email>] [--cc=<email>]
[--cover-letter] [--quiet]
[<common diff options>]
@@ -166,6 +166,14 @@ will want to ensure that threading is disabled for `git send-email`.
allows for useful naming of a patch series, and can be
combined with the `--numbered` option.
+--reroll-count=<n>::
+ Mark the series as the <n>-th iteration of the topic. The
+ output filenames have `v<n>` pretended to them, and the
+ subject prefix ("PATCH" by default, but configurable via the
+ `--subject-prefix` option) has ` v<n>` appended to it. E.g.
+ `--reroll-count=4` may produce `v4-0001-add-makefile.patch`
+ file that has "Subject: [PATCH v4 1/20] Add makefile" in it.
+
--to=<email>::
Add a `To:` header to the email headers. This is in addition
to any configured headers, and may be used multiple times.
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 959aa26..0ff9958 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -237,6 +237,14 @@ test_expect_success 'multiple files' '
ls patches/0001-Side-changes-1.patch patches/0002-Side-changes-2.patch patches/0003-Side-changes-3-with-n-backslash-n-in-it.patch
'
+test_expect_success 'reroll count' '
+ rm -fr patches &&
+ git format-patch -o patches --cover-letter --reroll-count 4 master..side >list &&
+ ! grep -v "^patches/v4-000[0-3]-" list &&
+ sed -n -e "/^Subject: /p" $(cat list) >subjects &&
+ ! grep -v "^Subject: \[PATCH v4 [0-3]/3\] " subjects
+'
+
check_threading () {
expect="$1" &&
shift &&
--
1.8.0.9.g5e84801
^ permalink raw reply related
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 22:28 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Jonathan Nieder, Junio C Hamano, Git Mailing List
In-Reply-To: <CACPiFCKNkpaf6CgU=5rn1dyUSG2KV43oeTKJgRsSh9-Rhtq3Kw@mail.gmail.com>
Martin Langhoff <martin.langhoff@gmail.com>:
> I dealt with enough CVS repos to see that the branch point could be
> ambiguous, and that some cases were incurably ugly and ambiguous.
You are quite right, but you have misintepreted the subject of my
confidence. I am under no illusion that the new cvsimport/cvsps
pair is a perfect solution to the CVS-lifting problem, nor even that
such a solution is possible.
> My best guess is that you haven't dealt with enough ugly CVS repos. I
> used to have the old original X.org repos, but no more. Surely
> Mozilla's fugly old CVS repos are up somewhere, and may be
> therapeutic.
Thanks, but since I wrote reposurgeon in 2010 I've done more conversions
of messy CVS and Subversion repositories than I can easily remember (the
Subversion ones being relevant because they often have truly nasty CVS
artifacts in their early history). Just off the top of my head there's
been gpsd, the Network Utility Tools, Roundup, SSTK2000, the Hercules
project, and robotfindskitten. And a raft of smaller projects - I sought
them out as torture tests for reposurgeon.
I am therefore intimately, painfully familiar with how bad CVS repos
can get. I take it as given that there are still boojums that will
perplex my tools lurking out there in the unexplored jungle.
In fact, this very kind of prior experience had been a major
motivation for reposurgeon. I became convinced several years back
that the batchy design philosophy of conventional repo-conversion
tools was flawed, not flexible enough to deal with the real-world
messes out there. So I wrote reposurgeon to amplify human judgment
rather than try to replace it.
An example of the batchiness mistake close to home is the -m and -M
options in the old version of cvsimport. It takes human judgment
looking at the whole commit DAG in gitspace to decide what merge
points would best express the (as you say, sometimes ambiguous) CVS
history - what's needed is a scalpel and sutures in a surgeon's hand,
not a regexp hammer.
For extended discussion, see my blog post "Repositories In
Translation" at http://esr.ibiblio.org/?p=3859 in which I argue that
the process has much more in common with the ambiguity of literary
translation than is normally understood.
No, what I am very confident about is the performance and stability of
the new cvsps/cvsimport code on *the cases the old code handled* - and
a fairly well-defined larger group of many more cases.
My confidence is derived from having built a test suite that
incorporates and improves on the git-tree tests. I don't have to merely
guess or hope that the new code works better, I can exhibit tests
that demonstrate it.
Among my near-term to-do items are applying those tests to cvs2git and
parsecvs. But I first need to get parsecvs working again; presently, as I've
inherited it, it does not correctly create a HEAD reference in the
translated git repo.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: git filter-branch doesn't dereference annotated tags
From: Grégory Pakosz @ 2013-01-02 22:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
In-Reply-To: <7vk3rwaa3r.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]
> I was wondering if it should be
>
> sha1=$(git rev-parse --verify "$ref")
>
> or something that does not dereference a tag at all.
>
> The way I read what that loop seems to want to do is:
>
> Read each refname that was given originally from the file
> $tempdir/heads, find out the object it used to refer to and
> have it in $sha1, find out what new object the object was
> rewritten to and have it in $rewritten, and:
>
> (1) if the rewrite left the object unchanged, do nothing but
> warn users just in case this was a mistake;
> (2) if the rewrite told us to remove it, then delete the
> ref; or
> (3) if the rewrite gave us a new object, replace the ref to
> point to that new one.
>
> And in the latter two cases, save the original one in
> $orig_namespace so that the user can choose to recover if
> this filter-branch was done by mistake.
>
> So I do not think unwraping the ref at that point makes any sense,
> unless it is not prepared to handle annotated tags at all by
> unwrapping tags too early.
>
> What am I missing?
>
So we have an annotated tag that points to a commit that is rewritten
to nothing as the result of the filtering. What should happen?
My initial questions and patching suggestions are based on git
1.7.10.4 behavior.
However, playing with git HEAD exhibits a slightly different behavior:
it breaks when invoking git mktag line 459 (introduced by
1bf6551e42c79a594689a356a9b14759d55f3cf5):
error: char7: could not get SHA1 hash
fatal: invalid tag signature file
Could not create new tag object for tag-a
It's basically the same problem. In my opinion, lines 447-466 should
take into account $new_sha1 is empty.
Please forgive me again for not having configured my mailer yet :(
When I'm ready to provide a patch that implements a solution we all
agree with I'll use git send-email.
In the mean time, I would like to pursue the discussion in this mail
thread so please find attached a patch that deletes a tag instead of
invoking the tag-name-filter when it detects $new_sha1 is empty.
I tested the patch doesn't break t7003. What do you think?
Gregory
[-- Attachment #2: 0001-git-filter-branch-allow-deletion-of-tags-when-refere.patch --]
[-- Type: application/octet-stream, Size: 2591 bytes --]
From 2cb9d5bc605cc2f2d8a6603b6a06657516959aa6 Mon Sep 17 00:00:00 2001
From: Gregory Pakosz <gpakosz@visionobjects.com>
Date: Wed, 2 Jan 2013 23:02:03 +0100
Subject: [PATCH] git-filter-branch: allow deletion of tags when referenced
commit gets rewritten to nothing
---
git-filter-branch.sh | 61 ++++++++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5314249..2e8569c 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -436,36 +436,41 @@ if [ "$filter_tag_name" ]; then
[ -f "../map/$sha1" ] || continue
new_sha1="$(cat "../map/$sha1")"
- GIT_COMMIT="$sha1"
- export GIT_COMMIT
- new_ref="$(echo "$ref" | eval "$filter_tag_name")" ||
- die "tag name filter failed: $filter_tag_name"
-
- echo "$ref -> $new_ref ($sha1 -> $new_sha1)"
-
- if [ "$type" = "tag" ]; then
- new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \
- "$new_sha1" "$new_ref"
- git cat-file tag "$ref" |
- sed -n \
- -e '1,/^$/{
- /^object /d
- /^type /d
- /^tag /d
- }' \
- -e '/^-----BEGIN PGP SIGNATURE-----/q' \
- -e 'p' ) |
- git mktag) ||
- die "Could not create new tag object for $ref"
- if git cat-file tag "$ref" | \
- sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
- then
- warn "gpg signature stripped from tag object $sha1t"
+ if [ -n "$new_sha1" ]; then
+ GIT_COMMIT="$sha1"
+ export GIT_COMMIT
+ new_ref="$(echo "$ref" | eval "$filter_tag_name")" ||
+ die "tag name filter failed: $filter_tag_name"
+
+ echo "$ref -> $new_ref ($sha1 -> $new_sha1)"
+
+ if [ "$type" = "tag" ]; then
+ new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \
+ "$new_sha1" "$new_ref"
+ git cat-file tag "$ref" |
+ sed -n \
+ -e '1,/^$/{
+ /^object /d
+ /^type /d
+ /^tag /d
+ }' \
+ -e '/^-----BEGIN PGP SIGNATURE-----/q' \
+ -e 'p' ) |
+ git mktag) ||
+ die "Could not create new tag object for $ref"
+ if git cat-file tag "$ref" | \
+ sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1
+ then
+ warn "gpg signature stripped from tag object $sha1t"
+ fi
fi
- fi
- git update-ref "refs/tags/$new_ref" "$new_sha1" ||
- die "Could not write tag $new_ref"
+ git update-ref "refs/tags/$new_ref" "$new_sha1" ||
+ die "Could not write tag $new_ref"
+ else
+ git update-ref -d "refs/tags/$ref" "$sha1t" ||
+ die "Could not delete tag $ref"
+ fi
done
fi
--
1.8.0.1
^ permalink raw reply related
* Re: Test failures with python versions when building git 1.8.1
From: Dan McGee @ 2013-01-02 21:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <7vehi34k5s.fsf@alter.siamese.dyndns.org>
On Wed, Jan 2, 2013 at 10:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Dan McGee <dan@archlinux.org> writes:
>
>> This works great now, thanks! I ran it through our package build
>> scripts and all tests now pass as expected.
>
> If you have a chance, could you try tip of the 'next' branch without
> this patch applied? We had an equivalent patch cooking there for
> some time by now.
Yeah, this worked great as well. Thanks again everyone.
-Dan
^ permalink raw reply
* Re: [PATCH] merge: Honor prepare-commit-msg return code
From: Junio C Hamano @ 2013-01-02 21:21 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git, Jay Soffian
In-Reply-To: <CALWbr2wWjwUnHFq1icMRuW=vjQDhTO1e_chffqUvDWY5za1Kiw@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
>>> prepare-commit-msg hook is run when committing to prepare the log
>>> message. If the exit-status is non-zero, the commit should be aborted.
>>
>> I was scratching my head why you CC'ed Jay, until I dug up 65969d4
>> (merge: honor prepare-commit-msg hook, 2011-02-14).
>
> I did as suggested in "SubmittingPatches" :)
Oh, that wasn't meant as a complaint. I am tempted to rewrite the
log message like so, though:
65969d4 (merge: honor prepare-commit-msg hook, 2011-02-14) tried to
make "git commit" and "git merge" consistent, because a merge that
required user assistance has to be concluded with "git commit", but
only "git commit" triggered prepare-commit-msg hook. When it added
a call to run the prepare-commit-msg hook, however, it forgot to
check the exit code from the hook like "git commit" does, and ended
up replacing one inconsistency with another.
>> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
>> index bc497bc..3573751 100755
>> --- a/t/t7505-prepare-commit-msg-hook.sh
>> +++ b/t/t7505-prepare-commit-msg-hook.sh
>> @@ -172,11 +172,12 @@ test_expect_success 'with failing hook (merge)' '
>> git checkout -B other HEAD@{1} &&
>> echo "more" >> file &&
>> git add file &&
>> - chmod -x $HOOK &&
>> + rm -f "$HOOK" &&
>> git commit -m other &&
>> - chmod +x $HOOK &&
>> + write_script "$HOOK" <<-EOF
>> + exit 1
>> + EOF
>> git checkout - &&
>> - head=`git rev-parse HEAD` &&
>> test_must_fail git merge other
>>
>> '
>
> What about moving the hook file then ? Not very important to me, just
> a suggestion as it would keep the shebang.
Strictly speaking, the way $HOOK is prepared in the original is
wrong. The script is always run under "#!/bin/sh" instead of the
shell the user told us to use with $SHELL_PATH. For a simple one
liner that only exits with 1, it does not matter, though.
Many test scripts got this wrong and that was the reason we later
added write_script helper function to the test suite.
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Martin Langhoff @ 2013-01-02 21:15 UTC (permalink / raw)
To: Eric Raymond; +Cc: Jonathan Nieder, Junio C Hamano, Git Mailing List
In-Reply-To: <20130102164107.GA19006@thyrsus.com>
On Wed, Jan 2, 2013 at 11:41 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Martin Langhoff <martin.langhoff@gmail.com>:
>> Replacement with something more solid is welcome, but until you are
>> extremely confident of its handling of legacy setups... I would still
>> provide the old cvsimport, perhaps in contrib.
>
> I am extremely confident. I built a test suite so I could be.
This is rather off-putting. Really.
I dealt with enough CVS repos to see that the branch point could be
ambiguous, and that some cases were incurably ugly and ambiguous.
Off the top of my head I can recall
- Files created on a branch appear on HEAD (if the cvs client was
well behaved, in HEAD's attic, if the cvs client was buggy... )
- Files tagged with the branch at a much later time. Scenario is a
developer opening/tagging a new branch mindlessly on a partial
checkout; then trying to "fix" the problem later.
My best guess is that you haven't dealt with enough ugly CVS repos. I
used to have the old original X.org repos, but no more. Surely
Mozilla's fugly old CVS repos are up somewhere, and may be
therapeutic.
cheers,
m
--
martin.langhoff@gmail.com
martin@laptop.org -- Software Architect - OLPC
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
^ permalink raw reply
* Re: [PATCH v3] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2013-01-02 21:10 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <7vfw2j2vlp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Zoltan Klinger <zoltan.klinger@gmail.com> writes:
>
>> +static const char* MSG_REMOVE = "Removing %s\n";
>> +static const char* MSG_WOULD_REMOVE = "Would remove %s\n";
>> +static const char* MSG_WOULD_NOT_REMOVE = "Would not remove %s\n";
I also noticed that this message is not used, which mwans that the
program used to say "Would not remove" for some paths but the
updated one will never do so.
^ permalink raw reply
* Re: [PATCH] merge: Honor prepare-commit-msg return code
From: Antoine Pelisse @ 2013-01-02 21:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jay Soffian
In-Reply-To: <7v623f2uam.fsf@alter.siamese.dyndns.org>
>> prepare-commit-msg hook is run when committing to prepare the log
>> message. If the exit-status is non-zero, the commit should be aborted.
>
> I was scratching my head why you CC'ed Jay, until I dug up 65969d4
> (merge: honor prepare-commit-msg hook, 2011-02-14).
I did as suggested in "SubmittingPatches" :)
>> +test_expect_success 'with failing hook (merge)' '
>> +
>> + git checkout -B other HEAD@{1} &&
>> + echo "more" >> file &&
>> + git add file &&
>> + chmod -x $HOOK &&
>
> I have a feeling that this will break folks without POSIXPERM
> prerequisite.
It felt wrong when I did it, but kept it consistent within the file.
It indeed looks older than other test files I've seen so far but I
don't feel like I know the test general-style-and-policy enough to fix
it myself either.
> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
> index bc497bc..3573751 100755
> --- a/t/t7505-prepare-commit-msg-hook.sh
> +++ b/t/t7505-prepare-commit-msg-hook.sh
> @@ -172,11 +172,12 @@ test_expect_success 'with failing hook (merge)' '
> git checkout -B other HEAD@{1} &&
> echo "more" >> file &&
> git add file &&
> - chmod -x $HOOK &&
> + rm -f "$HOOK" &&
> git commit -m other &&
> - chmod +x $HOOK &&
> + write_script "$HOOK" <<-EOF
> + exit 1
> + EOF
> git checkout - &&
> - head=`git rev-parse HEAD` &&
> test_must_fail git merge other
>
> '
What about moving the hook file then ? Not very important to me, just
a suggestion as it would keep the shebang.
Cheers,
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Jeff King @ 2013-01-02 20:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dan McGee, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <7vip7f4k7x.fsf@alter.siamese.dyndns.org>
On Wed, Jan 02, 2013 at 08:34:42AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Whether we end up doing something with contrib and tests or not, the
> > patch below gives a minimal fix in the meantime.
>
> Replacing the symbolic link with write_script that uses exported
> variables looks like a familiar pattern. I like it.
>
> Oh, wait. That pattern is of course familiar, because 5a02966
> (t9020: use configured Python to run the test helper, 2012-12-18)
> has been in 'next', and is planned for the first batch.
Great minds think alike, I guess?
Would have been nice to mention that you had done a patch when you
pointed to the unproductive thread. :P But I think you can take the
similarity of our patches and commit messages as my endorsement of
5a02966. :)
-Peff
^ permalink raw reply
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Stefano Lattarini @ 2013-01-02 20:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Jeff King, Martin von Zweigbergk, git
In-Reply-To: <7vbod72uze.fsf@alter.siamese.dyndns.org>
On 01/02/2013 09:25 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>
>>> ifdef AUTOCONFIGURED
>>> -config.status: configure
>>> - $(QUIET_GEN)if test -f config.status; then \
>>> +# We avoid depending on 'configure' here, because it gets rebuilt
>>> +# every time GIT-VERSION-FILE is modified, only to update the embedded
>>> +# version number string, which config.status does not care about.
>>>
>> Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
>> '@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
>> hard-coded in configure as the version number [1]. But if we do not
>> use those substitutions in any of our files (and I believe we don't),
>> then *we* can happily not care about the configure embedded version
>> number string, and thus avoid the extra configure runs. Phew.
>>
>> [1] Yes, this is a mess. We know. Sorry!
>
> Heh. Should we warn against the use of these symbols somewhere in
> configure.ac, perhaps, then?
>
Actually, they should be checked against in files processed by
'config.status', i.e., files listed in AC_CONFIG_FILES calls in
'configure.ac'. But I honestly believe that would be overkill;
I say we simply adjust your comment to read something like:
# We avoid depending on 'configure' here, because it gets rebuilt
# every time GIT-VERSION-FILE is modified, only to update the
# embedded version number string, which we however do not
# substitute in any file processed by config.status.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] merge: Honor prepare-commit-msg return code
From: Junio C Hamano @ 2013-01-02 20:40 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git, Jay Soffian
In-Reply-To: <1357152170-5511-1-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> prepare-commit-msg hook is run when committing to prepare the log
> message. If the exit-status is non-zero, the commit should be aborted.
I was scratching my head why you CC'ed Jay, until I dug up 65969d4
(merge: honor prepare-commit-msg hook, 2011-02-14).
> +test_expect_success 'with failing hook (merge)' '
> +
> + git checkout -B other HEAD@{1} &&
> + echo "more" >> file &&
> + git add file &&
> + chmod -x $HOOK &&
I have a feeling that this will break folks without POSIXPERM
prerequisite.
How about doing it this way instead? This old test script seems to
want a lot of style clean-ups, but I refrained from doing any in
this fixlet.
Thanks.
t/t7505-prepare-commit-msg-hook.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index bc497bc..3573751 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -172,11 +172,12 @@ test_expect_success 'with failing hook (merge)' '
git checkout -B other HEAD@{1} &&
echo "more" >> file &&
git add file &&
- chmod -x $HOOK &&
+ rm -f "$HOOK" &&
git commit -m other &&
- chmod +x $HOOK &&
+ write_script "$HOOK" <<-EOF
+ exit 1
+ EOF
git checkout - &&
- head=`git rev-parse HEAD` &&
test_must_fail git merge other
'
^ permalink raw reply related
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Junio C Hamano @ 2013-01-02 20:25 UTC (permalink / raw)
To: Stefano Lattarini; +Cc: Jonathan Nieder, Jeff King, Martin von Zweigbergk, git
In-Reply-To: <50E48BF6.2020900@gmail.com>
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>> ifdef AUTOCONFIGURED
>> -config.status: configure
>> - $(QUIET_GEN)if test -f config.status; then \
>> +# We avoid depending on 'configure' here, because it gets rebuilt
>> +# every time GIT-VERSION-FILE is modified, only to update the embedded
>> +# version number string, which config.status does not care about.
>>
> Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
> '@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
> hard-coded in configure as the version number [1]. But if we do not
> use those substitutions in any of our files (and I believe we don't),
> then *we* can happily not care about the configure embedded version
> number string, and thus avoid the extra configure runs. Phew.
>
> [1] Yes, this is a mess. We know. Sorry!
Heh. Should we warn against the use of these symbols somewhere in
configure.ac, perhaps, then?
^ permalink raw reply
* Re: [PATCH v3] git-clean: Display more accurate delete messages
From: Junio C Hamano @ 2013-01-02 20:11 UTC (permalink / raw)
To: Zoltan Klinger; +Cc: git
In-Reply-To: <1357091159-22080-1-git-send-email-zoltan.klinger@gmail.com>
Zoltan Klinger <zoltan.klinger@gmail.com> writes:
> +static const char* MSG_REMOVE = "Removing %s\n";
> +static const char* MSG_WOULD_REMOVE = "Would remove %s\n";
> +static const char* MSG_WOULD_NOT_REMOVE = "Would not remove %s\n";
> +static const char* MSG_WOULD_IGNORE_GIT_DIR = "Would ignore untracked git repository %s\n";
> +static const char* MSG_WARN_GIT_DIR_IGNORE = "ignoring untracked git repository %s";
> +static const char* MSG_WARN_REMOVE_FAILED = "failed to remove %s";
"foo* bar" should be "foo *bar". Also I personally find these
upcased message constants somewhat hard to read in the sites of use
in the code below.
Also gettext machinery needs to be told that these strings may be
asked to be replaced with their translations using _() elsewhere in
the code. I.e.
static const char *msg_remove = N_("Removing %s\n");
Aren't WOULD_IGNORE_GIT_DIR and WARN_GIT_DIR_IGNORE named somewhat
inconsistently? Perhaps the latter is WARN_IGNORED_GIT_DIR or
something?
> @@ -34,11 +42,109 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
> return 0;
> }
>
> +static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
> + int dry_run, int quiet, int *dir_gone)
> +{
> + DIR *dir;
> + struct strbuf quoted = STRBUF_INIT;
> + struct dirent *e;
> + int res = 0, ret = 0, gone = 1, original_len = path->len, len, i;
> + unsigned char submodule_head[20];
> + struct string_list dels = STRING_LIST_INIT_DUP;
> +
> + *dir_gone = 1;
> +
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
Shouldn't this be inside the next if() statement body? I also think
you could even omit this call when (!dry_run && quiet). The same
comment applies to all uses of quote_path_relative() in this patch,
including the ones that were kept from the original in clean.c,
which made sense because they were used in all if/else bodies that
followed them but this patch makes it no longer true.
> + if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> + !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
> + if (dry_run && !quiet)
> + printf(_(MSG_WOULD_IGNORE_GIT_DIR), quoted.buf);
> + else if (!dry_run)
> + warning(_(MSG_WARN_GIT_DIR_IGNORE), quoted.buf);
> +
> + *dir_gone = 0;
> + return 0;
> + }
> +
> + dir = opendir(path->buf);
> + if (!dir) {
> + /* an empty dir could be removed even if it is unreadble */
> + res = dry_run ? 0 : rmdir(path->buf);
> + if (res) {
> + warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
> + *dir_gone = 0;
> + }
> + return res;
> + }
> +
> + if (path->buf[original_len - 1] != '/')
> + strbuf_addch(path, '/');
> +
> + len = path->len;
> + while ((e = readdir(dir)) != NULL) {
> + struct stat st;
> + if (is_dot_or_dotdot(e->d_name))
> + continue;
> +
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, e->d_name);
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + if (lstat(path->buf, &st))
> + ; /* fall thru */
> + else if (S_ISDIR(st.st_mode)) {
> + if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
> + ret = 1;
> + if (gone)
> + string_list_append(&dels, quoted.buf);
> + else
> + *dir_gone = 0;
> + continue;
> + } else {
> + res = dry_run ? 0 : unlink(path->buf);
> + if (!res)
> + string_list_append(&dels, quoted.buf);
> + else {
> + warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
> + *dir_gone = 0;
> + ret = 1;
> + }
> + continue;
> + }
> +
> + /* path too long, stat fails, or non-directory still exists */
> + *dir_gone = 0;
> + ret = 1;
> + break;
> + }
> + closedir(dir);
> +
> + strbuf_setlen(path, original_len);
> + quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
> + if (*dir_gone) {
> + res = dry_run ? 0 : rmdir(path->buf);
> + if (!res)
> + *dir_gone = 1;
> + else {
> + warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
> + *dir_gone = 0;
> + ret = 1;
> + }
> + }
> +
> + if (!*dir_gone && !quiet) {
> + for (i = 0; i < dels.nr; i++)
> + printf(dry_run ? _(MSG_WOULD_REMOVE) : _(MSG_REMOVE), dels.items[i].string);
> + }
> + string_list_clear(&dels, 0);
> + return ret;
> +}
> int cmd_clean(int argc, const char **argv, const char *prefix)
> {
> - int i;
> - int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
> - int ignored_only = 0, config_set = 0, errors = 0;
> + int i, res;
> + int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
> + int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
> int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
> struct strbuf directory = STRBUF_INIT;
> struct dir_struct dir;
> @@ -49,7 +155,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> char *seen = NULL;
> struct option options[] = {
> OPT__QUIET(&quiet, N_("do not print names of files removed")),
> - OPT__DRY_RUN(&show_only, N_("dry run")),
> + OPT__DRY_RUN(&dry_run, N_("dry run")),
> OPT__FORCE(&force, N_("force")),
> OPT_BOOLEAN('d', NULL, &remove_directories,
> N_("remove whole directories")),
> @@ -77,7 +183,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> if (ignored && ignored_only)
> die(_("-x and -X cannot be used together"));
>
> - if (!show_only && !force) {
> + if (!dry_run && !force) {
> if (config_set)
> die(_("clean.requireForce set to true and neither -n nor -f given; "
> "refusing to clean"));
> @@ -150,38 +256,23 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> if (S_ISDIR(st.st_mode)) {
> strbuf_addstr(&directory, ent->name);
> qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> + if (remove_directories || (matches == MATCHED_EXACTLY)) {
> + if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))
> errors++;
> + if (gone && !quiet)
> + printf(dry_run ? _(MSG_WOULD_REMOVE) : _(MSG_REMOVE), qname);
> }
> strbuf_reset(&directory);
> } else {
> if (pathspec && !matches)
> continue;
> qname = quote_path_relative(ent->name, -1, &buf, prefix);
> + res = dry_run ? 0 : unlink(ent->name);
> + if (res) {
> + warning(_(MSG_WARN_REMOVE_FAILED), qname);
> errors++;
> - }
> + } else if (!quiet)
> + printf(dry_run ? _(MSG_WOULD_REMOVE) :_(MSG_REMOVE), qname);
spaces required around that ':' (ctx:WxV)
#313: FILE: builtin/clean.c:275:
+ printf(dry_run ? _(MSG_WOULD_REMOVE) :_(MSG_REMOVE), qname);
> }
> }
> free(seen);
The updated code structure is much nicer than the previous round,
but I am somewhat puzzled how return value of remove_dirs() and
&gone relate to each other. Surely when gone is set to zero,
remove_dirs() is reporting that the directory it was asked to remove
recursively did not go away, so it must report failure, no? Having
the &gone flag looks redundant and checking for gone in some places
while checking for the return value for others feels like an
invitation for future bugs.
Also the remove_dirs() function seems to replace the use of
remove_dir_recurse() from dir.c by copying large part of it, with
error message sprinkled. Does remove_dir_recurse() still get used
by other codepaths? If so, do the remaining callsites benefit from
using this updated version?
Thanks; will replace what has been sitting on the 'pu' branch with
this copy.
^ permalink raw reply
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Stefano Lattarini @ 2013-01-02 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Jeff King, Martin von Zweigbergk, git
In-Reply-To: <7va9sr4jgu.fsf@alter.siamese.dyndns.org>
On 01/02/2013 05:50 PM, Junio C Hamano wrote:
> Stefano Lattarini <stefano.lattarini@gmail.com> writes:
>
>> On 01/02/2013 09:48 AM, Jonathan Nieder wrote:
>>> Jeff King wrote:
>>>
>>>> It seems I am late to the party. But FWIW, this looks the most sane to
>>>> me of the patches posted in this thread.
>>> ...
>> FYI, this seems a sane approach to me....
>> The only nit I have to offer is that I'd like to see more comments in
>> the git Makefile about why this "semi-hack" is needed.
>
> Thanks, everybody.
>
> Please eyeball the below for (hopefully) the last time, to be
> eventually merged to maint-1.7.12, maint-1.8.0 and maint (aka
> maint-1.8.1) branches.
>
> -- >8 --
> From: Jonathan Nieder <jrnieder@gmail.com>
> Date: Wed, 2 Jan 2013 00:25:44 -0800
> Subject: [PATCH] build: do not automatically reconfigure unless configure.ac changed
>
> Starting with v1.7.12-rc0~4^2 (build: reconfigure automatically if
> configure.ac changes, 2012-07-19), "config.status --recheck" is
> automatically run every time the "configure" script changes. In
> particular, that means the configuration procedure repeats whenever
> the version number changes (since the configure script changes to
> support "./configure --version" and "./configure --help"), making
> bisecting painfully slow.
>
> The intent was to make the reconfiguration process only trigger for
> changes to configure.ac's logic. Tweak the Makefile rule to match
> that intent by depending on configure.ac instead of configure.
>
> Reported-by: Martin von Zweigbergk <martinvonz@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Reviewed-by: Jeff King <peff@peff.net>
> Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
> ---
> Makefile | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 26b697d..2f5e2ab 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE
> $(RM) $<+
>
> ifdef AUTOCONFIGURED
> -config.status: configure
> - $(QUIET_GEN)if test -f config.status; then \
> +# We avoid depending on 'configure' here, because it gets rebuilt
> +# every time GIT-VERSION-FILE is modified, only to update the embedded
> +# version number string, which config.status does not care about.
>
Alas, config.status *do* care about it, in that the '@PACKAGE_VERSION@',
'@PACKAGE_STRING@' and '@DEFS@' substitutions are affected by what is
hard-coded in configure as the version number [1]. But if we do not
use those substitutions in any of our files (and I believe we don't),
then *we* can happily not care about the configure embedded version
number string, and thus avoid the extra configure runs. Phew.
[1] Yes, this is a mess. We know. Sorry!
> +# We
> +# do want to recheck when the platform/environment detection logic
> +# changes, hence this depends on configure.ac.
> +config.status: configure.ac
> + $(QUIET_GEN)$(MAKE) configure && \
> + if test -f config.status; then \
> ./config.status --recheck; \
> else \
> ./configure; \
HTH,
Stefano
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Junio C Hamano @ 2013-01-02 19:07 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130102183710.GB19006@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> Junio C Hamano <gitster@pobox.com>:
>> As your version already knows how to detect the case where cvsps is
>> too old to operate with it, I imagine it to be straight-forward to
>> ship the old cvsimport under obscure name, "git cvsimport--old" or
>> something, and spawn it from your version when necessary, perhaps
>> after issuing a warning "cvsps 3.0 not found; switching to an old
>> and unmaintained version of cvsimport..."
>
> This can be done. As this may not be the last case in which it comes up,
> perhaps we should have an 'obsolete' directory distinct from 'contrib'.
>
> I'll ship another patch.
Alright; thanks.
Don't forget to sign-off your patch ;-)
^ permalink raw reply
* Re: [PATCH] merge: Honor prepare-commit-msg return code
From: Antoine Pelisse @ 2013-01-02 19:05 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Antoine Pelisse
In-Reply-To: <1357152170-5511-1-git-send-email-apelisse@gmail.com>
On Wed, Jan 2, 2013 at 7:42 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> prepare-commit-msg hook is run when committing to prepare the log
> message. If the exit-status is non-zero, the commit should be aborted.
>
> While the script is run before committing a successful merge, the
> exit-status is ignored and a non-zero exit doesn't abort the commit.
>
> Abort the commit if prepare-commit-msg returns with a non-zero status
> when committing a successful merge.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> builtin/merge.c | 5 +++--
> t/t7505-prepare-commit-msg-hook.sh | 13 +++++++++++++
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a96e8ea..3a31c4b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -800,8 +800,9 @@ static void prepare_to_commit(struct commit_list *remoteheads)
> if (0 < option_edit)
> strbuf_add_lines(&msg, "# ", comment, strlen(comment));
> write_merge_msg(&msg);
> - run_hook(get_index_file(), "prepare-commit-msg",
> - git_path("MERGE_MSG"), "merge", NULL, NULL);
> + if (run_hook(get_index_file(), "prepare-commit-msg",
> + git_path("MERGE_MSG"), "merge", NULL, NULL))
> + abort_commit(remoteheads, NULL);
> if (0 < option_edit) {
> if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
> abort_commit(remoteheads, NULL);
> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
> index 5b4b694..bc497bc 100755
> --- a/t/t7505-prepare-commit-msg-hook.sh
> +++ b/t/t7505-prepare-commit-msg-hook.sh
> @@ -167,5 +167,18 @@ test_expect_success 'with failing hook (--no-verify)' '
>
> '
>
> +test_expect_success 'with failing hook (merge)' '
> +
> + git checkout -B other HEAD@{1} &&
> + echo "more" >> file &&
> + git add file &&
> + chmod -x $HOOK &&
> + git commit -m other &&
> + chmod +x $HOOK &&
> + git checkout - &&
> + head=`git rev-parse HEAD` &&
The line above is useless ... Sorry about the noise.
> + test_must_fail git merge other
> +
> +'
>
> test_done
> --
> 1.8.1.rc3.27.g3b73c7d.dirty
>
^ permalink raw reply
* [PATCH] merge: Honor prepare-commit-msg return code
From: Antoine Pelisse @ 2013-01-02 18:42 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, Antoine Pelisse
prepare-commit-msg hook is run when committing to prepare the log
message. If the exit-status is non-zero, the commit should be aborted.
While the script is run before committing a successful merge, the
exit-status is ignored and a non-zero exit doesn't abort the commit.
Abort the commit if prepare-commit-msg returns with a non-zero status
when committing a successful merge.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
builtin/merge.c | 5 +++--
t/t7505-prepare-commit-msg-hook.sh | 13 +++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index a96e8ea..3a31c4b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -800,8 +800,9 @@ static void prepare_to_commit(struct commit_list *remoteheads)
if (0 < option_edit)
strbuf_add_lines(&msg, "# ", comment, strlen(comment));
write_merge_msg(&msg);
- run_hook(get_index_file(), "prepare-commit-msg",
- git_path("MERGE_MSG"), "merge", NULL, NULL);
+ if (run_hook(get_index_file(), "prepare-commit-msg",
+ git_path("MERGE_MSG"), "merge", NULL, NULL))
+ abort_commit(remoteheads, NULL);
if (0 < option_edit) {
if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
abort_commit(remoteheads, NULL);
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index 5b4b694..bc497bc 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -167,5 +167,18 @@ test_expect_success 'with failing hook (--no-verify)' '
'
+test_expect_success 'with failing hook (merge)' '
+
+ git checkout -B other HEAD@{1} &&
+ echo "more" >> file &&
+ git add file &&
+ chmod -x $HOOK &&
+ git commit -m other &&
+ chmod +x $HOOK &&
+ git checkout - &&
+ head=`git rev-parse HEAD` &&
+ test_must_fail git merge other
+
+'
test_done
--
1.8.1.rc3.27.g3b73c7d.dirty
^ permalink raw reply related
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 18:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4m331bn.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> As your version already knows how to detect the case where cvsps is
> too old to operate with it, I imagine it to be straight-forward to
> ship the old cvsimport under obscure name, "git cvsimport--old" or
> something, and spawn it from your version when necessary, perhaps
> after issuing a warning "cvsps 3.0 not found; switching to an old
> and unmaintained version of cvsimport..."
This can be done. As this may not be the last case in which it comes up,
perhaps we should have an 'obsolete' directory distinct from 'contrib'.
I'll ship another patch.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Junio C Hamano @ 2013-01-02 18:08 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130102003344.GA9651@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> If you try to use new git-cvsimport with old cvsps, old cvsps will complain
> of an invalid argument and git-cvsimport will quit.
I see an opening for smoother transition here.
Like it or not, you cannot force distros to ship with cvsps 3.0 when
we ship our 1.8.2 (or 2.0 or whatever) that includes a cvsimport
that requires cvsps 3.0. The best we can do is to make it capable
of working with cvsps 3.0 for a better result (when available), and
working with cvsps 2.0 in a limited way as ever (linear history
only, etc. etc.) when cvsps 3.0 is not available.
As your version already knows how to detect the case where cvsps is
too old to operate with it, I imagine it to be straight-forward to
ship the old cvsimport under obscure name, "git cvsimport--old" or
something, and spawn it from your version when necessary, perhaps
after issuing a warning "cvsps 3.0 not found; switching to an old
and unmaintained version of cvsimport..."
That way, people who have been happily working with linear CVS
histories with the old limited tool can keep using the same set-up
until their distro update their cvsps, without harming people who
need to work with more complex CVS histories, who can choose to
update their cvsps early themselves as $HOME/bin/cvsps earlier on
their $PATH.
By "cvsimport" (the current version), we are talking about a piece
of software that has been used in the field for more than 5 years,
still with a handful of patches to enhance it in the past two years.
A flag-day "this hot-off-the-press version is infinitely better"
replacement is not an option, especially when we can expect that
existing users are not asking for an "inifinitely better" version
(they rather prefer "stable" in the "works just as before" sense),
even when the hot-off-the-press version *is* infinitely better in
some use cases such as dealing with branchy histories.
^ permalink raw reply
* Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories
From: Junio C Hamano @ 2013-01-02 17:22 UTC (permalink / raw)
To: Jason Holden; +Cc: git, Jeff King
In-Reply-To: <7vsj6k5o1w.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jason Holden <jason.k.holden.swdev@gmail.com> writes:
>
>> Any reason to leave out the maintainers email addresses?
>
> Nothing particular, other than that I did not find anywhere in the
> file that does not break the flow.
I've prepared this on top of the previous three. The second hunk
that updates "(4) Sending your patches." is the logical place to
have this information.
The other hunks are correcting minor mistakes that are not related.
I think I'll squash them to the patch 3/3.
Documentation/SubmittingPatches | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index a7daad3..9e113d0 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -31,8 +31,9 @@ change is relevant to.
these parts should be based on their trees.
To find the tip of a topic branch, run "git log --first-parent
-master..pu" and look for the merge commit. The second parent of this
-commit is the tip of the topic branch.
+master..pu" and look for merge commits. The second parent of these
+commits are the tips of topic branches.
+
(1) Make separate commits for logically separate changes.
@@ -70,6 +71,7 @@ changes do not trigger errors with the sample pre-commit hook shipped
in templates/hooks--pre-commit. To help ensure this does not happen,
run git diff --check on your changes before you commit.
+
(2) Describe your changes well.
The first line of the commit message should be a short description (50
@@ -185,14 +187,20 @@ not a text/plain, it's something else.
Send your patch with "To:" set to the mailing list, with "cc:" listing
people who are involved in the area you are touching (the output from
"git blame $path" and "git shortlog --no-merges $path" would help to
-identify them), to solicit comments and reviews. After the list
-reached a consensus that it is a good idea to apply the patch, re-send
-it with "To:" set to the maintainer and "cc:" the list for inclusion.
-Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and
-"Tested-by:" after your "Signed-off-by:" line as necessary.
+identify them), to solicit comments and reviews.
+
+After the list reached a consensus that it is a good idea to apply the
+patch, re-send it with "To:" set to the maintainer [*1*] and "cc:" the
+list [*2*] for inclusion. Do not forget to add trailers such as
+"Acked-by:", "Reviewed-by:" and "Tested-by:" after your
+"Signed-off-by:" line as necessary.
+
+ [Addresses]
+ *1* The current maintainer: gitster@pobox.com
+ *2* The mailing list: git@vger.kernel.org
-(4) Sign your work
+(5) Sign your work
To improve tracking of who did what, we've borrowed the
"sign-off" procedure from the Linux kernel project on patches
@@ -304,7 +312,8 @@ suggests to the contributors:
even get them in a "on top of your change" patch form.
(3) Polish, refine, and re-send to the list and the people who
- spend their time to improve your patch. Go back to step (2).
+ spent their time to help improving your patch. Go back to
+ step (2) until step (4) happens.
(4) The list forms consensus that the last round of your patch is
good. Send it to the list and cc the maintainer.
^ permalink raw reply related
* Re: [PATCH] gitk: add a checkbox to control the visibility of tags
From: Junio C Hamano @ 2013-01-02 17:08 UTC (permalink / raw)
To: Lukasz Stelmach; +Cc: Paul Mackerras, git
In-Reply-To: <50E3E9C2.5040901@poczta.fm>
Lukasz Stelmach <stlman@poczta.fm> writes:
> W dniu 02.01.2013 08:24, Junio C Hamano pisze:
>> Paul Mackerras <paulus@samba.org> writes:
>>
>>> On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
>>>> Łukasz Stelmach <stlman@poczta.fm> writes:
>>>>
>>>>> Enable hiding of tags displayed in the tree as yellow labels.
>>>>> If a repository is used together with a system like Gerrit
>>>>> there may be quite a lot of tags used to control building
>>>>> and there may be hardly any place left for commit subjects.
>>>>>
>>>>> Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
>>>>> ---
>>>> ...
>>> If the concern is the amount of screen real-estate that the tags take
>>> up when there are many of them (which is a reasonable concern), I'd
>>> rather just put a single tag icon with "tags..." inside it and arrange
>>> to list all the tags in the diff display pane when the user clicks on
>>> it. I think that would be better than not showing the tags at all.
>>
>> Yeah, sounds very sensible. Thanks.
>
> I am afraid I don't really understand why tags should be listed in the
> diff pane only after clicking the "tags" tag (if this is what Junio has
> suggested)? How about just putting there another line saying: Tags, next
> to Parent and Chindren and all the stuff?
>
> If something should happen upon user interaction with the tag label a
> toolpit would be a better choince FWIW.
If you meant tooltip that shows extra information in a pop-up when
the user hovers the pointer, I think that would be a workable
solution, too, and probably is a better one, compared to showing it
in another pane that may or may not be selected.
^ permalink raw reply
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Martin von Zweigbergk @ 2013-01-02 17:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefano Lattarini, Jonathan Nieder, Jeff King, git
In-Reply-To: <7va9sr4jgu.fsf@alter.siamese.dyndns.org>
> diff --git a/Makefile b/Makefile
> index 26b697d..2f5e2ab 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE
> $(RM) $<+
>
> ifdef AUTOCONFIGURED
> -config.status: configure
> - $(QUIET_GEN)if test -f config.status; then \
> +# We avoid depending on 'configure' here, because it gets rebuilt
> +# every time GIT-VERSION-FILE is modified, only to update the embedded
> +# version number string, which config.status does not care about. We
> +# do want to recheck when the platform/environment detection logic
> +# changes, hence this depends on configure.ac.
> +config.status: configure.ac
> + $(QUIET_GEN)$(MAKE) configure && \
> + if test -f config.status; then \
> ./config.status --recheck; \
> else \
> ./configure; \
Looks great (at least from my 'make'-incompetent point of view :-)). I
do appreciate the comment. Thanks, everyone.
^ permalink raw reply
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Junio C Hamano @ 2013-01-02 16:50 UTC (permalink / raw)
To: Stefano Lattarini; +Cc: Jonathan Nieder, Jeff King, Martin von Zweigbergk, git
In-Reply-To: <50E4409B.4070203@gmail.com>
Stefano Lattarini <stefano.lattarini@gmail.com> writes:
> On 01/02/2013 09:48 AM, Jonathan Nieder wrote:
>> Jeff King wrote:
>>
>>> It seems I am late to the party. But FWIW, this looks the most sane to
>>> me of the patches posted in this thread.
>> ...
> FYI, this seems a sane approach to me....
> The only nit I have to offer is that I'd like to see more comments in
> the git Makefile about why this "semi-hack" is needed.
Thanks, everybody.
Please eyeball the below for (hopefully) the last time, to be
eventually merged to maint-1.7.12, maint-1.8.0 and maint (aka
maint-1.8.1) branches.
-- >8 --
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 2 Jan 2013 00:25:44 -0800
Subject: [PATCH] build: do not automatically reconfigure unless configure.ac changed
Starting with v1.7.12-rc0~4^2 (build: reconfigure automatically if
configure.ac changes, 2012-07-19), "config.status --recheck" is
automatically run every time the "configure" script changes. In
particular, that means the configuration procedure repeats whenever
the version number changes (since the configure script changes to
support "./configure --version" and "./configure --help"), making
bisecting painfully slow.
The intent was to make the reconfiguration process only trigger for
changes to configure.ac's logic. Tweak the Makefile rule to match
that intent by depending on configure.ac instead of configure.
Reported-by: Martin von Zweigbergk <martinvonz@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com>
---
Makefile | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 26b697d..2f5e2ab 100644
--- a/Makefile
+++ b/Makefile
@@ -2167,8 +2167,14 @@ configure: configure.ac GIT-VERSION-FILE
$(RM) $<+
ifdef AUTOCONFIGURED
-config.status: configure
- $(QUIET_GEN)if test -f config.status; then \
+# We avoid depending on 'configure' here, because it gets rebuilt
+# every time GIT-VERSION-FILE is modified, only to update the embedded
+# version number string, which config.status does not care about. We
+# do want to recheck when the platform/environment detection logic
+# changes, hence this depends on configure.ac.
+config.status: configure.ac
+ $(QUIET_GEN)$(MAKE) configure && \
+ if test -f config.status; then \
./config.status --recheck; \
else \
./configure; \
--
1.8.1.200.gd2acdf2
^ permalink raw reply related
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Thomas Berg @ 2013-01-02 16:48 UTC (permalink / raw)
To: esr; +Cc: Martin Langhoff, Jonathan Nieder, Junio C Hamano,
Git Mailing List
In-Reply-To: <20130102164107.GA19006@thyrsus.com>
On Wed, Jan 2, 2013 at 5:41 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Martin Langhoff <martin.langhoff@gmail.com>:
>> Replacement with something more solid is welcome, but until you are
>> extremely confident of its handling of legacy setups... I would still
>> provide the old cvsimport, perhaps in contrib.
>
> I am extremely confident. I built a test suite so I could be.
I too am glad to see some work go into the cvsimport script. So just
to clear things up, previously you said this:
> Yes, they must install an updated cvsps.
This is the problem, and one that is easily "solved" by just keeping a
copy of the old command.
Remember that for many users of these tools it doesn't matter if the
history is correct or not, as long as the head checkout contains the
right files and they are able to submit new changes. With this
definition of "works" git-cvsimport is not that broken I think.
Cheers,
- Thomas
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Andreas Schwab @ 2013-01-02 16:43 UTC (permalink / raw)
To: esr; +Cc: Jonathan Nieder, Junio C Hamano, git
In-Reply-To: <20130102161848.GA18447@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> Two of the three claims in this paragraph are false. The manual page
> does not tell you what is true, which is that old cvsps will fuck up
> every branch by putting the root point at the wrong place.
That doesn't look like being a widespread problem, or more people would
have complained.
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
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