* 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
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 16:41 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Jonathan Nieder, Junio C Hamano, Git Mailing List
In-Reply-To: <CACPiFCKDoAoKxM4YU6uKoOGcDgLbXnCoUMO5iyf-wCWXh3j70A@mail.gmail.com>
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.
--
<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: Martin Langhoff @ 2013-01-02 16:32 UTC (permalink / raw)
To: Eric Raymond; +Cc: Jonathan Nieder, Junio C Hamano, Git Mailing List
In-Reply-To: <20130102161848.GA18447@thyrsus.com>
First of all, I am at the same time a sad, nostalgic, and very happy
that old cvsimport is getting replaced.
On Wed, Jan 2, 2013 at 11:18 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> 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. And if you
> call silently and randomly damaging imports getting work done, your
> definitions of "work" and "done" are broken.
The existing cvsps/cvsimport combo work for CVS repos with simple
branches, and can track those over time.
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.
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] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Jonathan Nieder @ 2013-01-02 16:35 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: Junio C Hamano, git
In-Reply-To: <20130102161848.GA18447@thyrsus.com>
Eric S. Raymond wrote:
> Jonathan Nieder <jrnieder@gmail.com>:
>> The former is already loudly advertised in the package description and
>> manpage, at least lets you get work done, and works fine for simple
>> repositories with linear history.
>
> Two of the three claims in this paragraph are false.
Give me a break.
Are you telling me that when multiple users read a manpage that states
| WARNING: for certain situations the import leads to incorrect
| results. Please see the section ISSUES for further reference.
[...]
| Problems related to timestamps:
[...]
| Problems related to branches:
[...]
| Problems related to tags:
[...]
| consider using these alternative tools which proved to be more
| stable in practice:
and a package description that states
| The git cvsimport tool can incrementally import from a repository
| that is being actively developed and only requires remote access
| over CVS protocol. Unfortunately, in many situations the import
| leads to incorrect results. For reliable, one-shot imports, cvs2git
| from the cvs2svn package or parsecvs may be a better fit.
and decide to use the tool anyway, this is not evidence that the tool
is invaluable to them, despite its shortcomings?
Perhaps the users reporting bugs didn't read the manpage and package
description (despite quoting the same passages and explaining why they
used the command nonetheless) or I should ignore the judgement calls
they make.
Consider the following workflow:
1. Update imported project periodically using git-cvsimport
2. Hack, do code archaeology using "git log -S" and "git bisect",
etc.
3. Fall back to a web browser and cvsweb to confirm conclusions.
You are telling me that it is not a regression to change the workflow
to the following:
1. Try to use git-cvsimport.
2. Wonder where that command went.
Meanwhile Junio has already suggested a way out. Just rename the
command.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Junio C Hamano @ 2013-01-02 16:35 UTC (permalink / raw)
To: Dan McGee
Cc: Jeff King, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <CAEik5nMRAoHdx166Q7Zb5Yve6DiyVgN6EXQWGF=GgUtSyjiuSA@mail.gmail.com>
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.
Thanks.
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Junio C Hamano @ 2013-01-02 16:34 UTC (permalink / raw)
To: Jeff King
Cc: Dan McGee, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <20130102085935.GB9328@sigill.intra.peff.net>
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.
> -- >8 --
> Subject: [PATCH] t9020: don't run python from $PATH
>
> In t9020, we symlink in a python script from contrib to help
> with the testing. However, we don't munge its #!-line, which
> means we may run the wrong python (we want the one in
> PYTHON_PATH). On top of this, we use a symlink without
> checking the SYMLINKS prereq, and we fail to properly quote
> GIT_BUILD_DIR, which may have spaces.
>
> Instead of symlinking, let's just write a small script which
> will feed the contrib script to PYTHON_PATH. To avoid
> quoting issues, we just export the variables the script
> needs to run.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9020-remote-svn.sh | 5 ++++-
> t/test-lib.sh | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
> index 4f2dfe0..416623b 100755
> --- a/t/t9020-remote-svn.sh
> +++ b/t/t9020-remote-svn.sh
> @@ -14,7 +14,10 @@ export PATH="$HOME:$PATH"
>
> # We override svnrdump by placing a symlink to the svnrdump-emulator in .
> export PATH="$HOME:$PATH"
> -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
> +export GIT_BUILD_DIR
> +write_script svnrdump <<\EOF
> +exec "$PYTHON_PATH" "$GIT_BUILD_DIR"/contrib/svn-fe/svnrdump_sim.py "$@"
> +EOF
>
> init_git () {
> rm -fr .git &&
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f50f834..c17db19 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -45,7 +45,7 @@ fi
> fi
>
> . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> -export PERL_PATH SHELL_PATH
> +export PERL_PATH SHELL_PATH PYTHON_PATH
>
> # if --tee was passed, write the output not only to the terminal, but
> # additionally to the file test-results/$BASENAME.out, too.
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-02 16:28 UTC (permalink / raw)
To: Jeff King; +Cc: Torsten Bögershausen, git
In-Reply-To: <20130102094635.GD9328@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So taking all of that, a more idiomatic perl script would look something
> like:
>
> my $exit_code;
> sub err {
> my $msg = shift;
> print "$ARGV:$.: error: $msg: $_\n";
> $exit_code = 1;
> }
>
> while (<>) {
> chomp;
> if (/^\s*sed\s+-i/) {
> err('sed -i is not portable');
> }
> # ...and so on
>
> # this resets our $. for each file
> close ARGV if eof;
> }
> exit $exit_code;
>
> I'd personally probably write the conditions like:
>
> /^\s*sed\s+-i/ and err('sed -i is not portable');
>
> to make the structure of the program (i.e., a list of conditions to
> complain about) clear, but I know not everybody agrees with such a terse
> style.
Thanks for a nicely-done review.
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 16:18 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20130102153933.GA30813@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com>:
> The former is already loudly advertised in the package description and
> manpage, at least lets you get work done, and works fine for simple
> repositories with linear history.
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. And if you
call silently and randomly damaging imports getting work done, your
definitions of "work" and "done" are broken.
> Taking away a command that people have been using in everyday work is
> pretty much a textbook example of a regression, no?
That would be, but we are talking about replacing total breakage with
a git-cvsimport that actually works and that you invoke in pretty much the
same way as the old one. Nothing is or will be taken away.
In any case, once the distros package cvsps 3.x, old cvsimport will terminate
with an error return, because cvsps-3.x sees an obsolete option that
git-cvsimport tries to use as a command to treminate after displaying
a prompt to upgrade.
The most we can accomplish by being "conservative" is to lengthen the
window during which people will falsely believe that their conversion
process is working.
--
<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: Jonathan Nieder @ 2013-01-02 15:39 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: Junio C Hamano, git
In-Reply-To: <20130102105919.GA14391@thyrsus.com>
Eric S. Raymond wrote:
> Jonathan Nieder <jrnieder@gmail.com>:
>> Speaking with my Debian packager hat on: the updated cvsps is not
>> available in Debian. "git cvsimport" is, and it has users that report
>> bugs from time to time. With this change, I would either have to take
>> on responsibility for maintenance of the cvsps package (not going to
>> happen) or drop "git cvsimport". That's a serious regression.
>
> How does going from "it silently damages imports" to "it fails with
> an error message" constitute a regression?
The former is already loudly advertised in the package description and
manpage, at least lets you get work done, and works fine for simple
repositories with linear history.
Taking away a command that people have been using in everyday work is
pretty much a textbook example of a regression, no?
Hope that helps,
Jonathan
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Dan McGee @ 2013-01-02 14:18 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <20130102085935.GB9328@sigill.intra.peff.net>
On Wed, Jan 2, 2013 at 2:59 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Jan 01, 2013 at 11:18:46PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > [1] This symlink is doubly wrong, because any use of symbolic links
>> > in the test scripts needs to depend on the SYMLINKS prereq, and this
>> > does not.
>>
>> Yeah, I think we have discussed this once already in
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/210688/focus=210714
>
> Thanks for the pointer; it looks like nothing productive came of the
> earlier discussion. To give a hat trick of failure to this line of code,
> I notice that the existing code also does not properly put quotes around
> $GIT_BUILD_DIR.
>
>> > [2] In both the current code and what I showed above, the test scripts
>> > depend on things in contrib/. This is probably a bad idea in
>> > general, as the quality of what goes into contrib is not as closely
>> > watched (especially with respect to things like portability).
>> > Certainly I would not have known to look more carefully at a patch
>> > to contrib/svn-fe for breakage to the test suite.
>>
>> As long as such tests are made skippable with appropriate
>> prerequisites, I do not think it is bad to have their tests in t/; I
>> would say it is rather better than having them in contrib/ and leave
>> it not run by anybody, which happened to some of the stuff in
>> contrib/ already.
>
> Good point. While my sense of decorum wants to keep contrib totally
> split out, from a practical point of view, it is better to have more
> people run the tests and report failures than not.
>
> Whether we end up doing something with contrib and tests or not, the
> patch below gives a minimal fix in the meantime. Dan, does it fix your
> problem?
This works great now, thanks! I ran it through our package build
scripts and all tests now pass as expected.
Signed-off-by: Dan McGee <dan@archlinux.org>
> -- >8 --
> Subject: [PATCH] t9020: don't run python from $PATH
>
> In t9020, we symlink in a python script from contrib to help
> with the testing. However, we don't munge its #!-line, which
> means we may run the wrong python (we want the one in
> PYTHON_PATH). On top of this, we use a symlink without
> checking the SYMLINKS prereq, and we fail to properly quote
> GIT_BUILD_DIR, which may have spaces.
>
> Instead of symlinking, let's just write a small script which
> will feed the contrib script to PYTHON_PATH. To avoid
> quoting issues, we just export the variables the script
> needs to run.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9020-remote-svn.sh | 5 ++++-
> t/test-lib.sh | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
> index 4f2dfe0..416623b 100755
> --- a/t/t9020-remote-svn.sh
> +++ b/t/t9020-remote-svn.sh
> @@ -14,7 +14,10 @@ export PATH="$HOME:$PATH"
>
> # We override svnrdump by placing a symlink to the svnrdump-emulator in .
> export PATH="$HOME:$PATH"
> -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
> +export GIT_BUILD_DIR
> +write_script svnrdump <<\EOF
> +exec "$PYTHON_PATH" "$GIT_BUILD_DIR"/contrib/svn-fe/svnrdump_sim.py "$@"
> +EOF
>
> init_git () {
> rm -fr .git &&
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f50f834..c17db19 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -45,7 +45,7 @@ fi
> fi
>
> . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> -export PERL_PATH SHELL_PATH
> +export PERL_PATH SHELL_PATH PYTHON_PATH
>
> # if --tee was passed, write the output not only to the terminal, but
> # additionally to the file test-results/$BASENAME.out, too.
> --
> 1.8.1.rc3.4.gf3a2f57
>
^ permalink raw reply
* Re: [PATCH v2] build: do not automatically reconfigure unless configure.ac changed
From: Stefano Lattarini @ 2013-01-02 14:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jeff King, Martin von Zweigbergk, git, Junio C Hamano
In-Reply-To: <20130102084807.GB22919@elie.Belkin>
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.
>
> Thanks. config.status runs ./configure itself, though, so the rule
> should actually be
>
> config.status: configure.ac
> $(QUIET_GEN)$(MAKE) configure && \
> if test -f config.status; then \
> ./config.status --recheck; \
> else \
> ./configure;
> fi
>
> Rather than screw it up yet again, I'm going to sleep. :) If someone
> else corrects the patch before tomorrow, I won't mind.
>
FYI, this seems a sane approach to me. At least until Autoconf is
improved to offer better (read: some :-) support to "dynamic" package
version numbers specified at configure runtime. I hope that day
isn't too far, since the current Autoconf limitation has been causing
its share of annoyances small woes in Automake and Gnulib as well.
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,
Stefano
^ permalink raw reply
* Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API
From: Adam Spiers @ 2013-01-02 12:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7vobh8aans.fsf@alter.siamese.dyndns.org>
On Tue, Jan 1, 2013 at 8:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
>> index 0356d25..944fc39 100644
>> --- a/Documentation/technical/api-directory-listing.txt
>> +++ b/Documentation/technical/api-directory-listing.txt
>> @@ -9,8 +9,11 @@ Data structure
>> --------------
>>
>> `struct dir_struct` structure is used to pass directory traversal
>> -options to the library and to record the paths discovered. The notable
>> -options are:
>> +options to the library and to record the paths discovered. A single
>> +`struct dir_struct` is used regardless of whether or not the traversal
>> +recursively descends into subdirectories.
>
> I am somewhat lukewarm on this part of the change.
>
> The added "regardless of..." does not seem to add as much value as
> the two extra lines the patch spends. If we say something like:
>
> A `struct dir_struct` structure is used to pass options to
> traverse directories recursively, and to record all the
> paths discovered by the traversal.
>
> it might be much more direct and informative, I suspect, though.
I somewhat disagree ;) When I first encountered this code, I naturally
assumed that one struct would be created per sub-directory traversed.
This is after all a natural and very common design pattern. The point
of this hunk was to make it explicitly clear that this is *not* how it
works in dir.c. IMHO your rewording still contains a certain amount of
ambiguity in this regard. For example, it could mean that each
dir_struct records all the paths discovered underneath the sub-directory
it represents, and that these recursively bubble up to a top-level
dir_struct.
>> diff --git a/dir.c b/dir.c
>> index ee8e711..89e27a6 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -2,6 +2,8 @@
>> * This handles recursive filename detection with exclude
>> * files, index knowledge etc..
>> *
>> + * See Documentation/technical/api-directory-listing.txt
>> + *
>> * Copyright (C) Linus Torvalds, 2005-2006
>> * Junio Hamano, 2005-2006
>> */
>> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)
>> die("cannot use %s as an exclude file", fname);
>> }
>>
>> +/*
>> + * Loads the per-directory exclude list for the substring of base
>> + * which has a char length of baselen.
>> + */
>> static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
>> {
>> struct exclude_list *el;
>> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
>> (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
>> return; /* too long a path -- ignore */
>>
>> - /* Pop the ones that are not the prefix of the path being checked. */
>> + /* Pop the directories that are not the prefix of the path being checked. */
>
> The "one" does not refer to a "directory", but to an "exclude-list".
No, if that was the case, it would mean that multiple exclude lists
would be popped, but that is not the case here (prior to v4).
> Pop the ones that are not for parent directories of the path
> being checked
Better would be:
Pop the entries within the EXCL_DIRS exclude list which originate
from directories not in the prefix of the path being checked.
although as previously stated, the v4 series I have been holding off
from submitting (in order not to distract you from a maint release)
actually changes this behaviour so EXCL_DIRS becomes an exclude_group of
multiple exclude_lists, one per directory. So in v4, multiple
exclude_lists *will* be popped. I'll tweak the comment in v4 to make
this clear.
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 10:59 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20130102080247.GA20002@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com>:
> Speaking with my Debian packager hat on: the updated cvsps is not
> available in Debian. "git cvsimport" is, and it has users that report
> bugs from time to time. With this change, I would either have to take
> on responsibility for maintenance of the cvsps package (not going to
> happen) or drop "git cvsimport". That's a serious regression.
How does going from "it silently damages imports" to "it fails with
an error message" constitute a regression?
> The moment someone takes care of packaging the updated cvsps, I'll
> stop minding, though. ;-)
I'll ping the Debian QA group.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Jeff King @ 2013-01-02 9:46 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201301012240.10722.tboegi@web.de>
On Tue, Jan 01, 2013 at 10:40:08PM +0100, Torsten Bögershausen wrote:
> Add the perl script "check-non-portable-shell.pl" to detect non-portable
> shell syntax
Cool. Thanks for adding more test-lint. But...
> diff --git a/t/Makefile b/t/Makefile
> index 88e289f..7b0c4dc 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -23,7 +23,7 @@ TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
>
> all: $(DEFAULT_TEST_TARGET)
>
> -test: pre-clean $(TEST_LINT)
> +test: pre-clean test-lint-shell-syntax $(TEST_LINT)
> $(MAKE) aggregate-results-and-cleanup
I do not think it should be a hard-coded dependency of "test", as then
there is no way to turn it off. It would make more sense to me to set a
default TEST_LINT that includes it, but could be overridden by the user.
> prove: pre-clean $(TEST_LINT)
> @@ -43,7 +43,7 @@ clean-except-prove-cache:
> clean: clean-except-prove-cache
> $(RM) .prove
>
> -test-lint: test-lint-duplicates test-lint-executable
> +test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
This, however, is right. The point of "test-lint" is "use all the lint",
so adding it here makes sense (and anyone who has set "TEST_LINT =
test-lint" will get the new check).
> +test-lint-shell-syntax:
> + $(PERL_PATH) check-non-portable-shell.pl $(T)
This is wrong if $(PERL_PATH) contains spaces, no? Doing "$(PERL_PATH)"
is also wrong, because the expansion happens in 'make', and a
$(PERL_PATH) with double-quotes would fool the shell. Since we export
$PERL_PATH, I think doing:
"$$PERL_PATH"" check-non-portable-shell.pl $(T)
would be sufficient.
> --- /dev/null
> +++ b/t/check-non-portable-shell.pl
> @@ -0,0 +1,67 @@
> +#!/usr/bin/perl -w
This "-w" is ignored, since we execute by using $PERL_PATH. Maybe "use
warnings" instead?
> +sub check_one_file($) {
Perl subroutine prototypes are generally frowned on unless there is a
good reason to use them.
> + my $lineno=1;
Perl keeps track of this for you in the "$." variable.
> + my $filename=shift;
And if you use the automagic "<>" handle, this is already in $ARGV (but
note that you need to do a little bit of magic to make that work with
$.; see the entry for "$." in perlvar, and "eof" in perlfunc).
> + open(FINPUT, "<$filename") || die "Couldn't open filename $filename";
> + my @fdata = <FINPUT>;
> + close(FINPUT);
> +
> + while (my $line = shift @fdata) {
Not that our test scripts are so huge they won't fit into memory, but it
is generally good practice to loop on the handle rather than reading all
of the lines into an array.
> + do {
What's this do block for?
> + # sed -i
> + if ($line =~ /^\s*sed\s+-i/) {
> + printf("%s:%d:error: \"sed -i not portable\" %s\n", $filename, $lineno, $line);
> + $exitcode=1;
> + }
These would be a lot more readable if the printf was pulled out into a
helper function. And you can avoid the escaped quotes by using perl's qq
operator. E.g., qq(this string has "quotes" in it).
Also, putting a space before the "error:" matches what gcc outputs,
which some editors (e.g., vim) can recognize and let the user jump
straight to the error.
> +while (@ARGV) {
> + my $arg = shift @ARGV;
> + check_one_file($arg);
> +}
You can replace this with the magic <> filehandle.
So taking all of that, a more idiomatic perl script would look something
like:
my $exit_code;
sub err {
my $msg = shift;
print "$ARGV:$.: error: $msg: $_\n";
$exit_code = 1;
}
while (<>) {
chomp;
if (/^\s*sed\s+-i/) {
err('sed -i is not portable');
}
# ...and so on
# this resets our $. for each file
close ARGV if eof;
}
exit $exit_code;
I'd personally probably write the conditions like:
/^\s*sed\s+-i/ and err('sed -i is not portable');
to make the structure of the program (i.e., a list of conditions to
complain about) clear, but I know not everybody agrees with such a terse
style.
-Peff
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Joachim Schmitz @ 2013-01-02 9:09 UTC (permalink / raw)
To: git
In-Reply-To: <20130102085935.GB9328@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Jan 01, 2013 at 11:18:46PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> [1] This symlink is doubly wrong, because any use of symbolic links
>>> in the test scripts needs to depend on the SYMLINKS prereq, and
>>> this does not.
>>
>> Yeah, I think we have discussed this once already in
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/210688/focus=210714
>
> Thanks for the pointer; it looks like nothing productive came of the
> earlier discussion. To give a hat trick of failure to this line of
> code, I notice that the existing code also does not properly put
> quotes around $GIT_BUILD_DIR.
>
>>> [2] In both the current code and what I showed above, the test
>>> scripts depend on things in contrib/. This is probably a bad
>>> idea in general, as the quality of what goes into contrib is
>>> not as closely watched (especially with respect to things like
>>> portability). Certainly I would not have known to look more
>>> carefully at a patch to contrib/svn-fe for breakage to the test
>>> suite.
>>
>> As long as such tests are made skippable with appropriate
>> prerequisites, I do not think it is bad to have their tests in t/; I
>> would say it is rather better than having them in contrib/ and leave
>> it not run by anybody, which happened to some of the stuff in
>> contrib/ already.
>
> Good point. While my sense of decorum wants to keep contrib totally
> split out, from a practical point of view, it is better to have more
> people run the tests and report failures than not.
>
> Whether we end up doing something with contrib and tests or not, the
> patch below gives a minimal fix in the meantime. Dan, does it fix your
> problem?
>
> -- >8 --
> Subject: [PATCH] t9020: don't run python from $PATH
>
> In t9020, we symlink in a python script from contrib to help
> with the testing. However, we don't munge its #!-line, which
> means we may run the wrong python (we want the one in
> PYTHON_PATH). On top of this, we use a symlink without
> checking the SYMLINKS prereq, and we fail to properly quote
> GIT_BUILD_DIR, which may have spaces.
>
> Instead of symlinking, let's just write a small script which
> will feed the contrib script to PYTHON_PATH. To avoid
> quoting issues, we just export the variables the script
> needs to run.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> t/t9020-remote-svn.sh | 5 ++++-
> t/test-lib.sh | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
> index 4f2dfe0..416623b 100755
> --- a/t/t9020-remote-svn.sh
> +++ b/t/t9020-remote-svn.sh
> @@ -14,7 +14,10 @@ export PATH="$HOME:$PATH"
>
> # We override svnrdump by placing a symlink to the svnrdump-emulator
> in . export PATH="$HOME:$PATH"
With this patch that comment is no longer true.
> -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py "$HOME/svnrdump"
> +export GIT_BUILD_DIR
> +write_script svnrdump <<\EOF
> +exec "$PYTHON_PATH" "$GIT_BUILD_DIR"/contrib/svn-fe/svnrdump_sim.py
> "$@" +EOF
>
> init_git () {
> rm -fr .git &&
^ 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