* Re: [PATCH/RFC] Fix the result of "git grep -l -C <num>"
From: Thomas Rast @ 2012-01-23 16:08 UTC (permalink / raw)
To: Albert Yale; +Cc: git, rene.scharfe
In-Reply-To: <1327334484-35196-1-git-send-email-surfingalbert@gmail.com>
Albert Yale <surfingalbert@gmail.com> writes:
> When combining "git grep -l" with "-C <num>",
> the first result is omitted.
[...]
> For example, if the following command should output a list of 3
> different files (a.txt, b.txt, c.txt):
>
> $ git grep -l -C 1 albert_yale
> b.txt
> c.txt
Nice catch. Can you add a test?
Is -l the only option affected? Why isn't, for example, -L?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [Q] Determing if a commit is reachable from the HEAD ?
From: Junio C Hamano @ 2012-01-23 16:09 UTC (permalink / raw)
To: Brian Foster; +Cc: git mailing list, Sitaram Chamarty
In-Reply-To: <201201231020.04041.brian.foster@maxim-ic.com>
Brian Foster <brian.foster@maxim-ic.com> writes:
> I am probably misunderstanding ‘--quiet’, which the
> man page cryptically describes as “... allow the
> caller to test the exit status to see if a range
> of objects is fully connected (or not).” What is
> meant here by “fully connected” ?
If the real history looks like this:
---Y---x---HEAD
i.e. the commit at HEAD says "parent x" in it, and your lacks "x"
for whatever reason, Y..HEAD is not fully connected.
^ permalink raw reply
* Re: Finding all commits which modify a file
From: Neal Groothuis @ 2012-01-23 16:14 UTC (permalink / raw)
To: Neal Kreitzinger; +Cc: git
In-Reply-To: <4F1B4764.3010501@gmail.com>
> On 1/20/2012 3:35 PM, Neal Groothuis wrote:
>> I'm trying to find /all/ commits that change a file in the
>> repository...and its proving to be trickier than I thought. :-)
On 1/21/2012 6:16 PM, Neal Kreitzinger wrote:
> Does git-log --all help?
I don't see how it would. The commits are all reachable from HEAD, which
would seem to be the problem that --all would correct.
What I'm trying to do is find the commits in which a file differs from
that same file in any of its parents.
If I'm missing something, could you provide an example of using git-log
--all to accomplish this?
^ permalink raw reply
* [PATCH v2] Fix the result of "git grep [-l|-L] -C <num>"
From: Albert Yale @ 2012-01-23 17:06 UTC (permalink / raw)
To: git; +Cc: trast, rene.scharfe, Albert Yale
When combining "git grep [-l|-L]" with "-C <num>",
the first result is omitted.
Reviewed-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
builtin/grep.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..beebe20 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1036,7 +1036,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (use_threads) {
if (opt.pre_context || opt.post_context || opt.file_break ||
opt.funcbody)
- skip_first_line = 1;
+ {
+ if( ! ( opt.name_only || opt.unmatch_name_only ) )
+ skip_first_line = 1;
+ }
start_threads(&opt);
}
#endif
--
1.7.8.3
^ permalink raw reply related
* Re: [PATCH/RFC] Fix the result of "git grep -l -C <num>"
From: Albert Yale @ 2012-01-23 17:10 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, rene.scharfe
In-Reply-To: <8762g2ieq0.fsf@thomas.inf.ethz.ch>
You're right. This solution is incomplete. It needs to take into
account "opt.unmatch_name_only", and possibly other states. I'm open
for suggestions.
I've just posted Version 2 of this patch.
As for creating a test, I'm unfamiliar with the testing procedure for
git-core. A "how to" in the "Documentation" folder would be very
useful in that regard.
Albert
On Mon, Jan 23, 2012 at 11:08 AM, Thomas Rast <trast@student.ethz.ch> wrote:
> Albert Yale <surfingalbert@gmail.com> writes:
>
>> When combining "git grep -l" with "-C <num>",
>> the first result is omitted.
> [...]
>> For example, if the following command should output a list of 3
>> different files (a.txt, b.txt, c.txt):
>>
>> $ git grep -l -C 1 albert_yale
>> b.txt
>> c.txt
>
> Nice catch. Can you add a test?
>
> Is -l the only option affected? Why isn't, for example, -L?
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] grep: fix -l/-L interaction with decoration lines
From: Thomas Rast @ 2012-01-23 17:52 UTC (permalink / raw)
To: Albert Yale; +Cc: René Scharfe, git
In-Reply-To: <CALEc4zGV6Oo-WR0vPE6=oEmm=fo4dd=nyBWFuK1oU7rmF9K41A@mail.gmail.com>
From: Albert Yale <surfingalbert@gmail.com>
In threaded mode, git-grep emits file breaks (enabled with context, -W
and --break) into the accumulation buffers even if they are not
required. The output collection thread then uses skip_first_line to
skip the first such line in the output, which would otherwise be at
the very top.
This is wrong when the user also specified -l/-L/-c, in which case
every line is relevant. While arguably giving these options together
doesn't make any sense, git-grep has always quietly accepted it. So
do not skip anything in these cases.
Signed-off-by: Albert Yale <surfingalbert@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
> Reviewed-by: Thomas Rast <trast@student.ethz.ch>
Please don't. I didn't actually read the patch or look at the code,
or say so, and you're claiming I did. I was working purely from the
commit message.
> As for creating a test, I'm unfamiliar with the testing procedure for
> git-core. A "how to" in the "Documentation" folder would be very
> useful in that regard.
Well, there's t/README.
Here's a patch that also does -c and has tests. Placing them was more
finicky than I hoped; the list of files in the repo varies wildly
across the test set. It also exploits knowledge that git-ls-files
order is the same as 'git grep -l' order, which might not be
appropriate.
builtin/grep.c | 6 ++++--
t/t7810-grep.sh | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..1120b9f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1034,8 +1034,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
#ifndef NO_PTHREADS
if (use_threads) {
- if (opt.pre_context || opt.post_context || opt.file_break ||
- opt.funcbody)
+ if (!(opt.name_only || opt.unmatch_name_only ||
+ opt.count)
+ && (opt.pre_context || opt.post_context ||
+ opt.file_break || opt.funcbody))
skip_first_line = 1;
start_threads(&opt);
}
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 7ba5b16..75f4716 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -246,6 +246,28 @@ do
done
cat >expected <<EOF
+file
+EOF
+test_expect_success 'grep -l -C' '
+ git grep -l -C1 foo >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<EOF
+file:5
+EOF
+test_expect_success 'grep -l -C' '
+ git grep -c -C1 foo >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'grep -L -C' '
+ git ls-files >expected &&
+ git grep -L -C1 nonexistent_string >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<EOF
file:foo mmap bar_mmap
EOF
--
1.7.9.rc2.215.gd9e83
^ permalink raw reply related
* Re: git clone, hardlinks and multiple users?
From: Marc Herbert @ 2012-01-23 17:55 UTC (permalink / raw)
To: git
In-Reply-To: <jfc8eh$ck5$1@dough.gmane.org>
On 20/01/2012 17:31, Marc Herbert wrote:
> "git clone" is using hardlinks by default, even when cloning from a
> different user. In such a case the clone ends up with a number of files
> owned by someone else.
>
> Since only immutable objects are cloned this seems to work fine. However
> I would like to know if this "multiple users" case works by chance or by
> specification.
Sorry I meant: "since only immutable objects are HARDLINKED this seems
to work fine".
A few other clarifications following Neal's long answer:
- Yes we are using Linux. But the question is about any filesystem
supporting hardlinks and user permissions.
- My question is only about hardlinks in .git/objects/. Whatever happens
in the checkout is irrelevant.
- I know how to clone with no hardlink and completely avoid the whole
issue. Unfortunately people have this strange habit of using the
simplest/default option, and it does hardlinks.
I guess my rephrased question is: while there is no obvious reason for
git to attempt to touch files in .git/objects/, is there a promise that
this will never, ever happen? Because it would fail in a multi-users config.
The "core.sharedRepository" option is good example. When set to a new
value will it ever try to fix existing objects? That would fail.
^ permalink raw reply
* Re: [RFC/PATCH] git put: an alternative to add/reset/checkout
From: Junio C Hamano @ 2012-01-23 18:10 UTC (permalink / raw)
To: mike
Cc: Nguyen Thai Ngoc Duy, Jeff King, git, Scott Chacon,
Jakub Narebski, Matthieu Moy
In-Reply-To: <CADo4Y9j5MwKr+rWza0ncLWuthY6x+s68CQYbY2+c8-E5pAa=Sw@mail.gmail.com>
Michael Nahas <mike.nahas@gmail.com> writes:
> It obviously isn't the design-first-then-find-a-willing-programmer of
> the project I ran. I don't know if it's the IETF's "running code and
> a general consensus".
These two are not necessarily incompatible.
A proposed new feature needs to be explained well, describing in what
situation it will help what kind of users and without hurting others and
why it is a worthy addition.
We require a general consensus that any proposed change is a worthy
addition, and a working code is often a good addition to help us reaching
one, because people can guess what is being proposed even when the idea is
presented poorly. A poorly presented idea without working code often fares
no better than just a handwaving with crazy talk, as you fail to make
others realize what you are trying to achieve.
But working code is not a requirement to present good ideas. It just helps
to add clarity to it.
^ permalink raw reply
* Re: [PATCH] Fix typo in 1.7.9 release notes
From: Junio C Hamano @ 2012-01-23 18:11 UTC (permalink / raw)
To: mhagger; +Cc: git
In-Reply-To: <1327320598-28683-1-git-send-email-mhagger@alum.mit.edu>
Thanks.
^ permalink raw reply
* Re: [PATCH] grep: fix -l/-L interaction with decoration lines
From: Albert Yale @ 2012-01-23 18:28 UTC (permalink / raw)
To: Thomas Rast; +Cc: René Scharfe, git
In-Reply-To: <74777e0e8633d980fee9a1a680a63535be042fdc.1327340917.git.trast@student.ethz.ch>
Your patch is clearly better than mine. I'll let you take over this bug.
Thanks for taking a deeper look into this,
Albert Yale
On Mon, Jan 23, 2012 at 12:52 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> From: Albert Yale <surfingalbert@gmail.com>
>
> In threaded mode, git-grep emits file breaks (enabled with context, -W
> and --break) into the accumulation buffers even if they are not
> required. The output collection thread then uses skip_first_line to
> skip the first such line in the output, which would otherwise be at
> the very top.
>
> This is wrong when the user also specified -l/-L/-c, in which case
> every line is relevant. While arguably giving these options together
> doesn't make any sense, git-grep has always quietly accepted it. So
> do not skip anything in these cases.
>
> Signed-off-by: Albert Yale <surfingalbert@gmail.com>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
>> Reviewed-by: Thomas Rast <trast@student.ethz.ch>
>
> Please don't. I didn't actually read the patch or look at the code,
> or say so, and you're claiming I did. I was working purely from the
> commit message.
>
>> As for creating a test, I'm unfamiliar with the testing procedure for
>> git-core. A "how to" in the "Documentation" folder would be very
>> useful in that regard.
>
> Well, there's t/README.
>
>
> Here's a patch that also does -c and has tests. Placing them was more
> finicky than I hoped; the list of files in the repo varies wildly
> across the test set. It also exploits knowledge that git-ls-files
> order is the same as 'git grep -l' order, which might not be
> appropriate.
>
>
> builtin/grep.c | 6 ++++--
> t/t7810-grep.sh | 22 ++++++++++++++++++++++
> 2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..1120b9f 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1034,8 +1034,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>
> #ifndef NO_PTHREADS
> if (use_threads) {
> - if (opt.pre_context || opt.post_context || opt.file_break ||
> - opt.funcbody)
> + if (!(opt.name_only || opt.unmatch_name_only ||
> + opt.count)
> + && (opt.pre_context || opt.post_context ||
> + opt.file_break || opt.funcbody))
> skip_first_line = 1;
> start_threads(&opt);
> }
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 7ba5b16..75f4716 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -246,6 +246,28 @@ do
> done
>
> cat >expected <<EOF
> +file
> +EOF
> +test_expect_success 'grep -l -C' '
> + git grep -l -C1 foo >actual &&
> + test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> +file:5
> +EOF
> +test_expect_success 'grep -l -C' '
> + git grep -c -C1 foo >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'grep -L -C' '
> + git ls-files >expected &&
> + git grep -L -C1 nonexistent_string >actual &&
> + test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> file:foo mmap bar_mmap
> EOF
>
> --
> 1.7.9.rc2.215.gd9e83
>
^ permalink raw reply
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Junio C Hamano @ 2012-01-23 18:33 UTC (permalink / raw)
To: conrad.irwin; +Cc: git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <4f1d2a8b.a2d8320a.50ec.576d@mx.google.com>
conrad.irwin@gmail.com writes:
> ---8<---
>
> To set -grep on an file in .gitattributes will cause that file to be
> skipped completely while grepping. This can be used to reduce the number
> of false positives when your repository contains files that are
> uninteresting to you, for example test fixtures, dlls or minified source
> code.
Please reword this to describe the problem being solved first (why it
needs to be solved, in what situation you cannot live without the
feature), and then describe the approach you used to solve it.
Plain "grep" does this:
$ grep world hello.*
hello.c: printf("Hello, world.\n");
Binary file hello.o matches
in order to avoid uselessly spewing garbage at you while reminding you
that the command is not showing the whole truth and you _might_ want to
look into the elided file if you wanted to with "grep -a world hello.o".
Compared to this, it feels that the result your patch goes too far and
hides the truth a bit too much to my taste. Maybe it is just me.
Perhaps you, or all participants of your particular project, usually do
not want to see any grep hits from minified.js, but you may still want to
be able to say "I want to dig deeper and make sure I have copyright
strings in all files", for example. It is unclear how you envision to
support such a use case building on top of this patch.
Your "attributes only" is not an acceptable solution in the longer run,
even though it is a good first step (i.e. "attributes first and other
enhancement later"). There should be an easy way to get the best of both
worlds.
> The other approach considered was to allow an --exclude flag to grep at
> runtime, however that better serves the less common use-case of wanting
> to customise the list of files per-invocation rather than statically.
I doubt that it is justifiable to call per-invocation "the less common".
By the way, if the uninteresting ones are dll and minified.js, I wonder
why it is insufficient to mark them binary, i.e. uninteresting for the
purpose of all textual operations not just grep but also for diff.
I am *not* going to ask why they are treated as source and tracked by git
to begin with.
^ permalink raw reply
* Re: What does "modified" in git status mean?
From: Phillip Susi @ 2012-01-23 20:07 UTC (permalink / raw)
To: Mikolas; +Cc: git
In-Reply-To: <loom.20120122T174204-274@post.gmane.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 1/22/2012 11:57 AM, Mikolas wrote:
> I am using git version 1.7.5.1 under cygwin and I'm getting
> behavior that I'm not understanding.
>
> When I do 'git status' in the root directory of the repository, it
> shows no difference. Once I cd to a subdirectory, it starts showing
> modifications. However, 'git diff' shows nothing.
>
> So it looks something like this: $ git status # On branch master
> nothing to commit (working directory clean)
>
> $ cd foo $ git status # On branch master # Changes not staged for
> commit: # (use "git add <file>..." to update what will be
> committed) # (use "git checkout -- <file>..." to discard changes
> in working directory) # # modified: ../foo/bar
>
> $ git diff $
>
> I put the following in my gitconfig but that doesn't seem to be
> doing much. [core] trustctime = false autocrlf = input
autocrlf is basically broken, don't use it.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJPHb4EAAoJEJrBOlT6nu75NmsIALPYuRTdGPLigxmiAWfH4e/i
d516AR8AgkWtJzK3TbaD2HxgnQK9dmWYysppT7WR2SetrBrpsEUvQnw0b3jUewkb
lL97cEcDfiYbKXnkpfHRtAawUIXhrQeT/8XpOrvc7d8wPCWt4Zd7hrLo3TTuBGmN
tQcJtUqPwxreUYWOR5dPMV3oaeclptavEoeGc+2BlTiAuti6aw89G7lRvgVZQDGr
y0uCL4QIyOuMU9xUaeiFm/pCqY5MWSTs6Nv3mnAiw6PwK+aR1OcUkJjRw6dM6xWY
7futPNLEph8fptltaLhzMoeSzct3vNd+Al7synmOTh8bGitJeyaiCZOR55CG/eY=
=i8Zy
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: make install rewrites source files
From: Junio C Hamano @ 2012-01-23 20:15 UTC (permalink / raw)
To: Hallvard Breien Furuseth; +Cc: git
In-Reply-To: <hbf.20120123bz2f@bombur.uio.no>
Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> writes:
> INSTALL says we can install a profiled Git with
> $ make profile-all
> # make install prefix=...
> This does not work...
We should just drop prefix=... from that line, as the "prefix=value must
be the same while building and installing" is not only about the "profile"
build but applies to any other build.
I however wonder why you would need a separate profile-install target,
though. Shouldn't
$ make foo-build && make install
install a funky 'foo' variant of the build for any supported value of
'foo'?
^ permalink raw reply
* Re: make install rewrites source files
From: Phillip Susi @ 2012-01-23 20:28 UTC (permalink / raw)
To: Hallvard Breien Furuseth; +Cc: git
In-Reply-To: <hbf.20120123bz2f@bombur.uio.no>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:
> INSTALL says we can install a profiled Git with $ make profile-all
> # make install prefix=...
prefix should be an argument to configure, not make.
> This does not work: 'make install' notices that the build flags
> has changed and rebuilds Git - presumably without using the profile
> info. The patch below fixes this.
make install implicitly includes make all; it is supposed to rebuild
anything that needs rebuilt.
> However, make install should not write to the source directory in
> any case. That fails as root if root lacks write access there, due
> to NFS mounts that map root to nobody etc. At least git-instaweb
> and GIT-BUILD-OPTIONS are rewritten. You can simulate this with su
> nobody -s /bin/bash -c 'make -k install' after configuring with
> prefix=<directory owned by nobody>.
If you want to build locally from a read only nfs mount, then you
should run the configure script in a local directory:
mkdir /tmp/build
cd /tmp/build
/path/to/nfs/source/configure
make
make install
> Index: Makefile --- Makefile~ 2012-01-19 01:36:02.000000000 +0100
> +++ Makefile 2012-01-23 14:44:56.554980323 +0100
Hrm... Makefile should itself be a generated file from Makefile.in or
Makefile.am, but it appears that git isn't doing this. Perhaps that
should be fixed.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJPHcL3AAoJEJrBOlT6nu759U8IANCtJDWnCizSDWrJAFWe3ISr
FemiFgW347qjLcWlJS036nPfKnrxrJ88rF2e9+8Tj/hfPojNwCmyvN7rz+guI0uA
qqOfk9uN38Qd/jwfW5gv/7raKP4eUyRZ9ioptX3NqQtP5Co4TFuajOfswpN8f/DL
QiU7os62Df5HWW2U8A3XT9KiU9oWRala8dcrp5EJkEOfYDvQG2o3e1N/D91KC4el
lAyVEnzrvoLr5NzHCnFe7dQqvAB2S3PE/NP4anZHyNRp3SDLu1iZbD9MKC21Bd3n
BmCn9Vh7U+reC/NBMq8qaM69jLRk2Dx12brFoyY5/cjdQuaLj+n6h1nN9MqSKYI=
=/qLy
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: make install rewrites source files
From: Junio C Hamano @ 2012-01-23 20:52 UTC (permalink / raw)
To: Phillip Susi; +Cc: Hallvard Breien Furuseth, git
In-Reply-To: <4F1DC2F7.2070502@ubuntu.com>
Phillip Susi <psusi@ubuntu.com> writes:
> On 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:
>> INSTALL says we can install a profiled Git with $ make profile-all
>> # make install prefix=...
>
> prefix should be an argument to configure, not make.
What are you talking about? Use of ./configure is entirely optional, and
our Makefile _does_ support giving prefix on the command line.
^ permalink raw reply
* Re: make install rewrites source files
From: Hallvard Breien Furuseth @ 2012-01-23 20:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhazm89bo.fsf@alter.siamese.dyndns.org>
On Mon, 23 Jan 2012 12:15:07 -0800, Junio C Hamano <gitster@pobox.com> wrote:
> Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> writes:
>
>> INSTALL says we can install a profiled Git with
>> $ make profile-all
>> # make install prefix=...
>> This does not work...
>
> We should just drop prefix=... from that line, as the "prefix=value must
> be the same while building and installing" is not only about the "profile"
> build but applies to any other build.
Either add or remove a prefix so they match, yes. Fine by me either way.
> I however wonder why you would need a separate profile-install target,
> though. Shouldn't
>
> $ make foo-build && make install
>
> install a funky 'foo' variant of the build for any supported value of
> 'foo'?
'profile-all' makes 'all' with different CFLAGS from those in
Makefile. 'install' makes 'all' which notices CFLAGS has changed
since last build, so it rebuilds:
$ make install
* new build flags or prefix
...
That's also how the 2nd '$(MAKE) ... all' in profile-all can tell
that it should do anything. Thus my new 'profile-install:' target
with the same flags as the final $(MAKE) in profile-all.
This looks way too clever to me. 'make' can detect that flags have
changed, but should then fail (optionally?) instead of rebuilding.
That'd likely solve my issue with other files rewritten as root too.
But I'm not volunteering to rewrite your build system.
BTW, it'd be useful to split up 'profile-all' so it is possible
to ignore 'make test' failure and compilete the build anyway:
.PHONY: profile-all profile-clean profile-gen profile-use profile-install
profile-all: profile-clean profile-gen profile-use
profile-gen:
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all
$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test
profile-use:
$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all
--
Hallvard
^ permalink raw reply
* Re: [PATCH] add a Makefile switch to avoid gettext translation in shell scripts
From: Junio C Hamano @ 2012-01-23 22:01 UTC (permalink / raw)
To: Alex Riesen
Cc: Git Mailing List, Ævar Arnfjörð Bjarmason,
Jonathan Nieder
In-Reply-To: <20120119195222.GA5011@blimp.dmz>
Alex Riesen <raa.lkml@gmail.com> writes:
> Some systems have gettext.sh (GNU gettext) installed, but it is either broken
> or misconfigured in such a way so its output is not usable.
> For instance, on this particular system, a Cygwin installations gettext
> produces no output whatsoever.
>
> In case the users of these systems are unable or not interested in fixing
> them, setting the new Makefile switch should help:
>
> USE_FALLTHROUGH_GETTEXT_SCHEME=yes
>
> This will replace the translation routines with fallthrough versions, which
> currently used only for regression testing.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
I'll split this into two, the first one only to restructure the code and
the guts of this one that avoids the autodetection.
Also I'd rename this so that:
$ make USE_GETTEXT_SCHEME=fallthrough
$ make USE_GETTEXT_SCHEME=gnu
could be used to avoid extra and unnecessary runtime overhead when the
person building git knows what is on the system.
^ permalink raw reply
* [PATCH 1/2] git-sh-i18n: restructure the logic to compute gettext.sh scheme
From: Junio C Hamano @ 2012-01-23 22:02 UTC (permalink / raw)
To: Git Mailing List
Cc: Alex Riesen, Ævar Arnfjörð Bjarmason,
Jonathan Nieder
In-Reply-To: <7v1uqq84es.fsf@alter.siamese.dyndns.org>
Instead of having a single long and complex chain of commands to decide
what to do and carry out the decision, split the code so that we first
decide which scheme to use, and in the second section define what exactly
is done by the chosen scheme. It makes the code much easier to follow and
update.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-sh-i18n.sh | 103 +++++++++++++++++++++++++++-----------------------------
1 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index b4575fb..6648bd3 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -16,61 +16,45 @@ else
fi
export TEXTDOMAINDIR
-if test -z "$GIT_GETTEXT_POISON"
+# First decide what scheme to use...
+GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
+if test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+then
+ : no probing necessary
+elif test -n "$GIT_GETTEXT_POISON"
then
- if test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && type gettext.sh >/dev/null 2>&1
- then
- # This is GNU libintl's gettext.sh, we don't need to do anything
- # else than setting up the environment and loading gettext.sh
- GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
- export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
- # Try to use libintl's gettext.sh, or fall back to English if we
- # can't.
- . gettext.sh
-
- elif test -z "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" && test "$(gettext -h 2>&1)" = "-h"
- then
- # We don't have gettext.sh, but there's a gettext binary in our
- # path. This is probably Solaris or something like it which has a
- # gettext implementation that isn't GNU libintl.
- GIT_INTERNAL_GETTEXT_SH_SCHEME=solaris
- export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
- # Solaris has a gettext(1) but no eval_gettext(1)
- eval_gettext () {
- gettext "$1" | (
- export PATH $(git sh-i18n--envsubst --variables "$1");
- git sh-i18n--envsubst "$1"
- )
- }
-
- else
- # Since gettext.sh isn't available we'll have to define our own
- # dummy pass-through functions.
-
- # Tell our tests that we don't have the real gettext.sh
- GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
- export GIT_INTERNAL_GETTEXT_SH_SCHEME
-
- gettext () {
- printf "%s" "$1"
- }
-
- eval_gettext () {
- printf "%s" "$1" | (
- export PATH $(git sh-i18n--envsubst --variables "$1");
- git sh-i18n--envsubst "$1"
- )
- }
- fi
-else
- # Emit garbage under GETTEXT_POISON=YesPlease. Unlike the C tests
- # this relies on an environment variable
-
GIT_INTERNAL_GETTEXT_SH_SCHEME=poison
- export GIT_INTERNAL_GETTEXT_SH_SCHEME
+elif type gettext.sh >/dev/null 2>&1
+then
+ # GNU libintl's gettext.sh
+ GIT_INTERNAL_GETTEXT_SH_SCHEME=gnu
+elif test "$(gettext -h 2>&1)" = "-h"
+then
+ # gettext binary exists but no gettext.sh. likely to be a gettext
+ # binary on a Solaris or something that is not GNU libintl and
+ # lack eval_gettext.
+ GIT_INTERNAL_GETTEXT_SH_SCHEME=gettext_without_eval_gettext
+fi
+export GIT_INTERNAL_GETTEXT_SH_SCHEME
+# ... and then follow that decision.
+case "$GIT_INTERNAL_GETTEXT_SH_SCHEME" in
+gnu)
+ # Use libintl's gettext.sh, or fall back to English if we can't.
+ . gettext.sh
+ ;;
+gettext_without_eval_gettext)
+ # Solaris has a gettext(1) but no eval_gettext(1)
+ eval_gettext () {
+ gettext "$1" | (
+ export PATH $(git sh-i18n--envsubst --variables "$1");
+ git sh-i18n--envsubst "$1"
+ )
+ }
+ ;;
+poison)
+ # Emit garbage so that tests that incorrectly rely on translatable
+ # strings will fail.
gettext () {
printf "%s" "# GETTEXT POISON #"
}
@@ -78,7 +62,20 @@ else
eval_gettext () {
printf "%s" "# GETTEXT POISON #"
}
-fi
+ ;;
+*)
+ gettext () {
+ printf "%s" "$1"
+ }
+
+ eval_gettext () {
+ printf "%s" "$1" | (
+ export PATH $(git sh-i18n--envsubst --variables "$1");
+ git sh-i18n--envsubst "$1"
+ )
+ }
+ ;;
+esac
# Git-specific wrapper functions
gettextln () {
--
1.7.9.rc2.48.g92994
^ permalink raw reply related
* [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Junio C Hamano @ 2012-01-23 22:04 UTC (permalink / raw)
To: Git Mailing List
Cc: Alex Riesen, Ævar Arnfjörð Bjarmason,
Jonathan Nieder
In-Reply-To: <7vwr8i6prk.fsf_-_@alter.siamese.dyndns.org>
From: Alex Riesen <raa.lkml@gmail.com>
Some systems have gettext.sh (GNU gettext) installed, but it is either
broken or misconfigured in such a way so its output is not usable. In
case the users of these systems are unable or not interested in fixing
them, setting the new Makefile switch should help:
make USE_GETTEXT_SCHEME=fallthrough
This will replace the translation routines with fallthrough versions,
that does not use gettext from the platform.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 4 ++++
git-sh-i18n.sh | 5 ++++-
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 9470a10..4435854 100644
--- a/Makefile
+++ b/Makefile
@@ -47,6 +47,9 @@ all::
# A translated Git requires GNU libintl or another gettext implementation,
# plus libintl-perl at runtime.
#
+# Define USE_GETTEXT_SCHEME and set it to 'fallthrough', if you don't trust
+# the installed gettext translation of the shell scripts output.
+#
# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't
# trust the langinfo.h's nl_langinfo(CODESET) function to return the
# current character set. GNU and Solaris have a nl_langinfo(CODESET),
@@ -1874,6 +1877,7 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
-e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+ -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \
-e $(BROKEN_PATH_FIX) \
$@.sh >$@+
endef
diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh
index 6648bd3..d5fae99 100644
--- a/git-sh-i18n.sh
+++ b/git-sh-i18n.sh
@@ -18,7 +18,10 @@ export TEXTDOMAINDIR
# First decide what scheme to use...
GIT_INTERNAL_GETTEXT_SH_SCHEME=fallthrough
-if test -n "$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
+if test -n "@@USE_GETTEXT_SCHEME@@"
+then
+ GIT_INTERNAL_GETTEXT_SH_SCHEME="@@USE_GETTEXT_SCHEME@@"
+elif test -n "@@USE_FALLTHROUGH_GETTEXT_SCHEME@@$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS"
then
: no probing necessary
elif test -n "$GIT_GETTEXT_POISON"
--
1.7.9.rc2.48.g92994
^ permalink raw reply related
* Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Jonathan Nieder @ 2012-01-23 22:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git Mailing List, Alex Riesen,
Ævar Arnfjörð Bjarmason
In-Reply-To: <7vr4yq6poy.fsf_-_@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> make USE_GETTEXT_SCHEME=fallthrough
>
> This will replace the translation routines with fallthrough versions,
> that does not use gettext from the platform.
Nice implementation. I still don't understand why NO_GETTEXT=YesPlease
should not imply this. Is it to ensure the GETTEXT_SCHEME=gnu mode
gets more testing?
Here's a patch to consider squashing in that makes the option take
effect if it changes between builds.
diff --git i/Makefile w/Makefile
index 63dfd64d..b2b738bb 100644
--- i/Makefile
+++ w/Makefile
@@ -2268,7 +2268,7 @@ cscope:
### Detect prefix changes
TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):\
$(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\
- $(localedir_SQ)
+ $(localedir_SQ):$(USE_GETTEXT_SCHEME)
GIT-CFLAGS: FORCE
@FLAGS='$(TRACK_CFLAGS)'; \
^ permalink raw reply related
* [PATCH] merge: use editor by default in interactive sessions
From: Junio C Hamano @ 2012-01-23 22:18 UTC (permalink / raw)
To: git
Traditionally, a cleanly resolved merge was committed by "git merge" using
the auto-generated merge commit log message with invoking the editor.
After 5 years of use in the field, it turns out that people perform too
many unjustified merges of the upstream history into their topic branches.
These merges are not just useless, but they are often not explained well,
and making the end result unreadable when it gets time for merging their
history back to their upstream.
Earlier we added the "--edit" option to the command, so that people can
edit the log message to explain and justify their merge commits. Let's
take it one step further and spawn the editor by default when we are in an
interactive session (i.e. the standard input and the standard output are
pointing at the same tty device).
There may be existing scripts that leave the standard input and the
standard output of the "git merge" connected to whatever environment the
scripts were started, and such invocation might trigger the above
"interactive session" heuristics. GIT_MERGE_AUTOEDIT environment variable
can be set to "no" at the beginning of such scripts to use the historical
behaviour while the script runs.
Note that this backward compatibility is meant only for scripts, and we
deliberately do *not* support "merge.edit = yes/no/auto" configuration
option to allow people to keep the historical behaviour.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* With an update to the documentation, so that I won't forget, even
though this topic came too late in the cycle and not meant for the
upcoming 1.7.9.
Documentation/git-merge.txt | 2 +-
Documentation/merge-options.txt | 16 +++++++++++++---
builtin/merge.c | 38 ++++++++++++++++++++++++++++++++++----
t/test-lib.sh | 3 ++-
4 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index e2e6aba..3ceefb8 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -9,7 +9,7 @@ git-merge - Join two or more development histories together
SYNOPSIS
--------
[verse]
-'git merge' [-n] [--stat] [--no-commit] [--squash]
+'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
[-s <strategy>] [-X <strategy-option>]
[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
'git merge' <msg> HEAD <commit>...
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 6bd0b04..f2f1d0f 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -8,10 +8,20 @@ failed and do not autocommit, to give the user a chance to
inspect and further tweak the merge result before committing.
--edit::
--e::
+--no-edit::
+ Invoke an editor before committing successful mechanical merge to
+ further edit the auto-generated merge message, so that the user
+ can explain and justify the merge. The `--no-edit` option can be
+ used to accept the auto-generated message (this is generally
+ discouraged). The `--edit` option is still useful if you are
+ giving a draft message with the `-m` option from the command line
+ and want to edit it in the editor.
+
- Invoke editor before committing successful merge to further
- edit the default merge message.
+Older scripts may depend on the historical behaviour of not allowing the
+user to edit the merge log message. They will see an editor opened when
+they run `git merge`. To make it easier to adjust such scripts to the
+updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
+set to `no` at the beginning of them.
--ff::
--no-ff::
diff --git a/builtin/merge.c b/builtin/merge.c
index 99f1429..0006175 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -46,7 +46,7 @@ static const char * const builtin_merge_usage[] = {
static int show_diffstat = 1, shortlog_len, squash;
static int option_commit = 1, allow_fast_forward = 1;
-static int fast_forward_only, option_edit;
+static int fast_forward_only, option_edit = -1;
static int allow_trivial = 1, have_message;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
@@ -189,7 +189,7 @@ static struct option builtin_merge_options[] = {
"create a single commit instead of doing a merge"),
OPT_BOOLEAN(0, "commit", &option_commit,
"perform a commit if the merge succeeds (default)"),
- OPT_BOOLEAN('e', "edit", &option_edit,
+ OPT_BOOL('e', "edit", &option_edit,
"edit message before committing"),
OPT_BOOLEAN(0, "ff", &allow_fast_forward,
"allow fast-forward (default)"),
@@ -877,12 +877,12 @@ static void prepare_to_commit(void)
write_merge_msg(&msg);
run_hook(get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL, NULL);
- if (option_edit) {
+ if (0 < option_edit) {
if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
abort_commit(NULL);
}
read_merge_msg(&msg);
- stripspace(&msg, option_edit);
+ stripspace(&msg, 0 < option_edit);
if (!msg.len)
abort_commit(_("Empty commit message."));
strbuf_release(&merge_msg);
@@ -1076,6 +1076,33 @@ static void write_merge_state(void)
close(fd);
}
+static int default_edit_option(void)
+{
+ static const char name[] = "GIT_MERGE_AUTOEDIT";
+ const char *e = getenv(name);
+ struct stat st_stdin, st_stdout;
+
+ if (have_message)
+ /* an explicit -m msg without --[no-]edit */
+ return 0;
+
+ if (e) {
+ int v = git_config_maybe_bool(name, e);
+ if (v < 0)
+ die("Bad value '%s' in environment '%s'", e, name);
+ return v;
+ }
+
+ /* Use editor if stdin and stdout are the same and is a tty */
+ return (!fstat(0, &st_stdin) &&
+ !fstat(1, &st_stdout) &&
+ isatty(0) &&
+ st_stdin.st_dev == st_stdout.st_dev &&
+ st_stdin.st_ino == st_stdout.st_ino &&
+ st_stdin.st_mode == st_stdout.st_mode);
+}
+
+
int cmd_merge(int argc, const char **argv, const char *prefix)
{
unsigned char result_tree[20];
@@ -1261,6 +1288,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
}
+ if (option_edit < 0)
+ option_edit = default_edit_option();
+
if (!use_strategies) {
if (!remoteheads->next)
add_strategies(pull_twohead, DEFAULT_TWOHEAD);
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..ed32c2a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -63,7 +63,8 @@ GIT_AUTHOR_NAME='A U Thor'
GIT_COMMITTER_EMAIL=committer@example.com
GIT_COMMITTER_NAME='C O Mitter'
GIT_MERGE_VERBOSITY=5
-export GIT_MERGE_VERBOSITY
+GIT_MERGE_AUTOEDIT=no
+export GIT_MERGE_VERBOSITY GIT_MERGE_AUTOEDIT
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
export EDITOR
--
1.7.9.rc2.48.g92994
^ permalink raw reply related
* Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Junio C Hamano @ 2012-01-23 22:23 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Git Mailing List, Alex Riesen,
Ævar Arnfjörð Bjarmason
In-Reply-To: <20120123221256.GG20833@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Junio C Hamano wrote:
>
>> make USE_GETTEXT_SCHEME=fallthrough
>>
>> This will replace the translation routines with fallthrough versions,
>> that does not use gettext from the platform.
>
> Nice implementation. I still don't understand why NO_GETTEXT=YesPlease
> should not imply this.
Should be easy to do so, like this?
diff --git a/Makefile b/Makefile
index a782409..c4c1066 100644
--- a/Makefile
+++ b/Makefile
@@ -1521,6 +1521,7 @@ ifdef GETTEXT_POISON
endif
ifdef NO_GETTEXT
BASIC_CFLAGS += -DNO_GETTEXT
+ USE_GETTEXT_SCHEME = fallthrough
endif
ifdef NO_STRCASESTR
COMPAT_CFLAGS += -DNO_STRCASESTR
^ permalink raw reply related
* Re: [PATCH] merge: use editor by default in interactive sessions
From: Michael Haggerty @ 2012-01-23 22:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vipk26p1b.fsf@alter.siamese.dyndns.org>
On 01/23/2012 11:18 PM, Junio C Hamano wrote:
> Traditionally, a cleanly resolved merge was committed by "git merge" using
> the auto-generated merge commit log message with invoking the editor.
s/with/without/
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] merge: use editor by default in interactive sessions
From: Junio C Hamano @ 2012-01-23 22:34 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <4F1DDECB.5080904@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 01/23/2012 11:18 PM, Junio C Hamano wrote:
>> Traditionally, a cleanly resolved merge was committed by "git merge" using
>> the auto-generated merge commit log message with invoking the editor.
>
> s/with/without/
Oops, thanks, ... goes to find a hole to crawl into ...
^ permalink raw reply
* Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import
From: Pete Wyckoff @ 2012-01-23 22:40 UTC (permalink / raw)
To: Vitor Antunes; +Cc: Luke Diamand, Junio C Hamano, git
In-Reply-To: <CAOpHH-W1LY3Q50otrcNJTYWN67k_pCZHEOkgbKy7kPgfUbGeQw@mail.gmail.com>
vitor.hda@gmail.com wrote on Mon, 23 Jan 2012 14:01 +0000:
> On Sat, Jan 21, 2012 at 5:11 PM, Pete Wyckoff <pw@padd.com> wrote:
> > luke@diamand.org wrote on Sat, 21 Jan 2012 10:51 +0000:
> >> However, one thing I noticed in reading through is that it will
> >> break if you end up importing a P4 branch that has spaces (or other
> >> shell chars) in its name. A quick test confirms this.
> >>
> >> - the code doesn't handle the names properly
> >> - git and p4 have different ideas about valid branch names
> >>
> >> But before rejecting Vitor's changes because of that it would be
> >> worth considering whether we care (much). My own opinion is that if
> >> you have developers who are daft enough to put spaces or dollars in
> >> their branch names then their project is already doomed anyway....
> >>
> >> Perhaps it would be enough just to issue a warning ("your project is
> >> doomed; start working on your CV") and skip such branch names rather
> >> than falling over with inexplicable error messages.
> >
> > This doesn't seem like a big deal. The read_pipe and
> > read_pipe_lines calls shoud be list-ified. That gets rid
> > of the problem with shell interactions.
> >
> > For git branch name reserved characters, a little function
> > to replace the bogus characters with "_" would avoid needing
> > to go work on the resume. Anything in bad_ref_char() and
> > check_refname_component(). I agree this doesn't have to be
> > perfect.
> >
> > This could be a new patch unrelated to Vitor's series, which
> > verifies branch names anywhere a new commit is made.
>
> I would also prefer to include that fix on a separate patch series that
> would include the test case Luke already prepared. In my opinion,
> updating read_pipe and read_pipe_lines is out of scope for the current
> patch series.
How about taking what's below and just squashing it in. It's
incremental on your changes and would go well with Luke's series
that fixes a bunch of scattered quoting issues similarly.
The change to "describe %s" is unnecessary, but makes all the
invocations look similar. You can leave it out.
This may conflict if you've already factored out the big
"if self.detectBranches" chunk into a separate function as
Junio recommended.
> BTW, and on an unrelated topic, are any test cases failing on your side?
I do run the tests regularly, and your series is good. There's
the 'clone --use-client-spec' one that is broken until my
2ea09b5 (git-p4: adjust test to adhere to stricter useClientSpec,
2012-01-11) is merged. It's on pu.
-- Pete
-----------8<-------------------
From f1cfb3836f5150dca86238225da56fe0bd577df8 Mon Sep 17 00:00:00 2001
From: Pete Wyckoff <pw@padd.com>
Date: Thu, 10 Nov 2011 07:40:14 -0500
Subject: [PATCH] git-p4: use list invoctaions to avoid shell mangling
Change git and p4 command invocations to avoid going through
the shell. This allows branch names with spaces and wildcards
to work.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2e3b741..b440966 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1961,7 +1961,7 @@ class P4Sync(Command, P4UserMap):
def importChanges(self, changes):
cnt = 1
for change in changes:
- description = p4Cmd("describe %s" % change)
+ description = p4Cmd(["describe", str(change)])
self.updateOptionDict(description)
if not self.silent:
@@ -2022,9 +2022,9 @@ class P4Sync(Command, P4UserMap):
self.commit(description, filesForCommit, tempBranch, [branchPrefix])
self.tempBranches.append(tempBranch)
self.checkpoint()
- for blob in read_pipe_lines("git rev-list --reverse --no-merges %s" % parent):
+ for blob in read_pipe_lines(["git", "rev-list", "--reverse", "--no-merges", parent]):
blob = blob.strip()
- if len( read_pipe("git diff-tree %s %s" % (blob, tempBranch)) ) == 0:
+ if len(read_pipe(["git", "diff-tree", blob, tempBranch])) == 0:
parentFound = True
if self.verbose:
print "Found parent of %s in commit %s" % (branch, blob)
--
1.7.9.rc2.33.g492ae
^ permalink raw reply related
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