Git development
 help / color / mirror / Atom feed
* Re: [RFC/PATCH] git put: an alternative to add/reset/checkout
From: Michael Nahas @ 2012-01-23 13:53 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Jeff King, git, Scott Chacon, Jakub Narebski, Matthieu Moy,
	Junio C Hamano
In-Reply-To: <CACsJy8BCGi3s8gXr4kk-u8tDWztV6ozg1Tap23Q=TxA5d9iL+g@mail.gmail.com>

On Mon, Jan 23, 2012 at 5:32 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>
> (Bringing up an old thread)

"...thank you so much for bringing up such a painful subject. While
you're at it, why don't you give me a nice paper cut and pour lemon
juice on it."


"git put" is "git cp".  It copies from one filesystem (or a snapshot
of a filesystem) to another filesystem.

Without multiple working directories, a modifiable "stash", or a
(useful) name for the filesystem referred to as
"index"/"cache"/"staging area", there is only one filesystem that the
command can write to: the (singular) working directory.

So, "git put <src filesystem> -- <path>" is fine.  It will copy from
the path in the src filesystem to the path in the current working
directory.  I don't think the command "put" is a great name for that.
Since we already have some strange double-usage commands like "git
checkout --" and "git reset --", perhaps this should be "git
cherry-pick --".

<rant>
But for my money, "git cp" is clearer and I'd love to get rid of the
user-confusing double-usage commands.  I'd replace "git checkout --"
with "git cp NEXT WTREE -- <path>" and replace "git reset --" with
"git cp HEAD NEXT --" where NEXT is the filesystem represented by the
"index"/"cache"/"staging area" and WTREE is an alias for the working
directory.
</rant>

Still, good luck.  It's a useful addition even if it is "git cherry-pick --".

Mike

>
>
> On Wed, Jun 8, 2011 at 3:06 AM, Jeff King <peff@peff.net> wrote:
> > ...
> > But another way to think about it is that commits, the index, and the
> > working tree are all "locations" with content. And one common operation
> > you may want to do is to move content from one spot to another, either
> > whole, by file, or by diff hunks. To a new user, knowing that "add" is
> > the command for moving content from thet working tree to the index does
> > not help them know which command to use to do the opposite content
> > movement.
> > ...
> > My idea is therefore to have a single command for moving content from
> > one location to another. You specify a source and a destination and get
> > a uniform interface for moving content.
> >
> > A proof-of-concept patch is below. Be aware that is meant to be
> > illustrative and is not well tested. Also, it is a minimal presentation
> > of the concept. Other "locations" may also be meaningful. I'll include
> > some ideas below the patch.
> >
> > ---
> >  Makefile   |    1 +
> >  git-put.sh |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 71 insertions(+), 0 deletions(-)
> >  create mode 100644 git-put.sh
> >
> > diff --git a/Makefile b/Makefile
> > index e40ac0c..4564506 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -368,6 +368,7 @@ SCRIPT_SH += git-merge-one-file.sh
> >  SCRIPT_SH += git-merge-resolve.sh
> >  SCRIPT_SH += git-mergetool.sh
> >  SCRIPT_SH += git-pull.sh
> > +SCRIPT_SH += git-put.sh
> >  SCRIPT_SH += git-quiltimport.sh
> >  SCRIPT_SH += git-rebase.sh
> >  SCRIPT_SH += git-repack.sh
> > diff --git a/git-put.sh b/git-put.sh
> > new file mode 100644
> > index 0000000..f673e14
> > --- /dev/null
> > +++ b/git-put.sh
> > @@ -0,0 +1,70 @@
> > +#!/bin/sh
> > +
> > +SUBDIRECTORY_OK=Yes
> > +OPTIONS_KEEPDASHASH=Yes
> > +OPTIONS_SPEC="\
> > +git put [options] <from> <to> [--] <file...>
> > +
> > +Move contents from one place to another, where <from> and <to> are one of:
> > +  1. A commit (e.g., master, HEAD~10, v1.7.5)
> > +  2. The special token INDEX to indicate git's index.
> > +  3. The special token WORKTREE to indicate the working directory.
> > +
> > +Options:
> > +--
> > +p            don't move whole files; use the patch interface
> > +"
> > +. git-sh-setup
> > +
> > +patch=
> > +while test $# != 0; do
> > +       case "$1" in
> > +       -p) patch=--patch ;;
> > +       --) shift; break ;;
> > +       *) usage ;;
> > +       esac
> > +       shift
> > +done
> > +test $# -lt 2 && usage
> > +
> > +from=$1; shift
> > +to=$1; shift
> > +test "$1" = "--" && shift
> > +
> > +type_of() {
> > +       case "$1" in
> > +       INDEX) echo index ;;
> > +       WORKTREE) echo worktree ;;
> > +       *) echo commit ;;
> > +       esac
> > +}
> > +
> > +# Checkout contents to worktree without munging the index in
> > +# between.
> > +worktree_checkout() {
> > +       old=$GIT_INDEX_FILE
> > +       test -z "$old" && old=$(git rev-parse --git-dir)/index
> > +       new=$(git rev-parse --git-dir)/put-index.tmp
> > +       cp "$old" "$new" &&
> > +       GIT_INDEX_FILE=$new git checkout "$@"
> > +       status=$?
> > +       rm -f "$new"
> > +       exit $status
> > +}
> > +
> > +case "$(type_of "$from"),$(type_of "$to")" in
> > +*,commit)
> > +       die "You can't modify an existing commit." ;;
> > +index,index)
> > +       die "You can't move content from the index on top of itself." ;;
> > +worktree,index)
> > +       exec git add $patch -- "$@" ;;
> > +commit,index)
> > +       exec git reset $patch "$from" -- "$@" ;;
> > +index,worktree)
> > +       exec git checkout $patch -- "$@" ;;
> > +worktree,worktree)
> > +       die "You can't move content in the worktree on top of itself." ;;
> > +commit,worktree)
> > +       worktree_checkout $patch "$from" -- "$@" ;;
> > +esac
> >
> >
> > As you can see, this handles only three typoes of locations: the
> > worktree, the index, and an arbitrary commit (really a tree-ish).
>
> Last time we were stuck at the magic keywords INDEX and WORKTREE. What
> if we sort of follow scp naming convention:
>
>  - Normal paths are working tree's paths
>  - Paths with a colon in it are in "remote" locations (index or a
> tree). The part before colon specifies the location.
>
> We could have:
>
> git put <src> [<src>...] <dst>
> git put <src> [<src>...] <dst> -- <pathspec>
>
> Where <src> and <dst> could be
>
>  - <tree-ish> <colon> [<pathspec>]
>  - [0-3] <colon> [<pathspec>]
>  - <pathspec> (or plain path)
>
> In the first form, pathspec could be specified in <src>. If <dst> is
> worktree, then "." would be enough (or path to repo's root to be more
> strict). In the second form, no pathspec can be part of <src> nor
> <dst> because they're at the end already.
>
> With this syntax we could have:
>
> git put 0:path/to/file.c . (or git put 0: path/to/file.c)
>  -> copy file.c from index to worktree (at the same path "path/to/file.c")
> git put path/to/file 0:
>  -> copy file to index
> git put HEAD: . -- path/
>  -> checkout everything in path/ from HEAD
>
> I'm not sure how mutiple <src> should work, but there may be a use case for it.
>
> > Some other types I've thought of are:
> > ...
> >  - branches as destinations; obviously we can't change an existing
> >    commit, but what about something like:
> >
> >      git put WORKTREE BRANCH:foo
> >
> >    to optionally create a new branch "refs/heads/foo" based on the
> >    current HEAD, push changes into a temporary index that matches its
> >    tip, and then making a new commit based on top.
> >
> >    This would serve a similar purpose to stashes, except that they
> >    would be named and could operate as full branches. I would find it
> >    useful for picking apart a mass of worktree changes into discrete
> >    commits.
> >
> >  - allow multiple destinations, like
> >
> >     # equivalent to "git checkout --"
> >     git put HEAD INDEX,WORKTREE
>
> These obviously do not work with the syntax I propose.
> --
> Duy

^ permalink raw reply

* Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import
From: Vitor Antunes @ 2012-01-23 14:01 UTC (permalink / raw)
  To: Pete Wyckoff; +Cc: Luke Diamand, Junio C Hamano, git
In-Reply-To: <20120121171130.GA6235@padd.com>

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.

BTW, and on an unrelated topic, are any test cases failing on your side?

Thanks,
Vitor

^ permalink raw reply

* make install rewrites source files
From: Hallvard Breien Furuseth @ 2012-01-23 14:18 UTC (permalink / raw)
  To: git

INSTALL says we can install a profiled Git with
	$ make profile-all
	# make install prefix=...
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.

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>.


Index: INSTALL
--- INSTALL~
+++ INSTALL
@@ -29,6 +29,6 @@ If you're willing to trade off (much) lo
 faster git you can also do a profile feedback build with
 
-	$ make profile-all
-	# make prefix=... install
+	$ make profile-all     prefix=...
+	# make profile-install prefix=...
 
 This will run the complete test suite as training workload and then
Index: Makefile
--- Makefile~	2012-01-19 01:36:02.000000000 +0100
+++ Makefile	2012-01-23 14:44:56.554980323 +0100
@@ -2695,5 +2695,5 @@ cover_db_html: cover_db
 ### profile feedback build
 #
-.PHONY: profile-all profile-clean
+.PHONY: profile-all profile-clean profile-install
 
 PROFILE_GEN_CFLAGS := $(CFLAGS) -fprofile-generate -DNO_NORETURN=1
@@ -2708,2 +2708,5 @@ profile-all: profile-clean
 	$(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test
 	$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all
+
+profile-install:
+	$(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" install

^ permalink raw reply

* Re: [RFC/PATCH] git put: an alternative to add/reset/checkout
From: Nguyen Thai Ngoc Duy @ 2012-01-23 14:35 UTC (permalink / raw)
  To: mike
  Cc: Jeff King, git, Scott Chacon, Jakub Narebski, Matthieu Moy,
	Junio C Hamano
In-Reply-To: <CADo4Y9iH+J-X-TdqTN2Y9KhQnprnCVvC4Xy6qhVHwsBRmsZUrg@mail.gmail.com>

On Mon, Jan 23, 2012 at 8:53 PM, Michael Nahas <mike.nahas@gmail.com> wrote:
> "git put" is "git cp".  It copies from one filesystem (or a snapshot
> of a filesystem) to another filesystem.

Exactly.

> Without multiple working directories, a modifiable "stash", or a
> (useful) name for the filesystem referred to as
> "index"/"cache"/"staging area", there is only one filesystem that the
> command can write to: the (singular) working directory.

No there are two writable "filesystems": working directory and
"index/cache/staging area"

> So, "git put <src filesystem> -- <path>" is fine.  It will copy from
> the path in the src filesystem to the path in the current working
> directory.  I don't think the command "put" is a great name for that.
> Since we already have some strange double-usage commands like "git
> checkout --" and "git reset --", perhaps this should be "git
> cherry-pick --".

The "-- <path>" thing may save you a few keystrokes when you want to
copy from more than one path(spec). The two below commands are
equivalent

git put HEAD:a/ HEAD/b/ HEAD/c/ .
git put HEAD: . -- a/ b/ c/

But of course if you just need to copy from one pathspec to another
place, "--" syntax is redundant.

> <rant>
> But for my money, "git cp" is clearer and I'd love to get rid of the
> user-confusing double-usage commands.  I'd replace "git checkout --"
> with "git cp NEXT WTREE -- <path>" and replace "git reset --" with
> "git cp HEAD NEXT --" where NEXT is the filesystem represented by the
> "index"/"cache"/"staging area" and WTREE is an alias for the working
> directory.
> </rant>

I thought of "cp" (naturally, I was driven by "scp" syntax as I said)
and maybe if we think this through, we may be able to enhance cp to
support "remote locations" (and --patch option). So "put" vs "cp" is
not important to me now. What I'd like to hear is whether the syntax
makes sense.

My "hidden" plan if this works out would be to deprecate (or
discourage) everything in git-checkout except branch switching. I
don't have anything against git-reset. It's a kind of dangerous
command from the start (while git-checkout is more user friendly) and
can stay that way. The new "git <cp, put or whatever name>" should
fill 90% the needs for git-reset.
-- 
Duy

^ permalink raw reply

* Re: [RFC/PATCH] git put: an alternative to add/reset/checkout
From: Michael Nahas @ 2012-01-23 14:56 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Jeff King, git, Scott Chacon, Jakub Narebski, Matthieu Moy,
	Junio C Hamano
In-Reply-To: <CACsJy8AB-6b_PMvyM7hRV3b=5o0Cn4CtosygUQOevTzVJhU=hg@mail.gmail.com>

Hi Duy,

I've contributed no code to git.  I've come up with plenty of ideas,
which seem to have gotten little traction.

Your ideas are similar to mine (and others), but the last attempt to
get them into git did not accomplish anything.  I don't know how much
work you have done on git, but before participating with git again, I
suggest you look at why the last attempt failed and we ask an
experienced person how things work.

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".  The only thing I've found is that people did
not want to discuss theory.  (I believe the feeling is that theory is
only worthy of DARCS.)  I also got the feeling that improving the user
interface (e.g., replacing "git checkout --" and "git reset --") was
not a priority.

So, please plan out a strategy before recruiting me to help push this
idea forward.

Mike


On Mon, Jan 23, 2012 at 9:35 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>
> On Mon, Jan 23, 2012 at 8:53 PM, Michael Nahas <mike.nahas@gmail.com> wrote:
> > "git put" is "git cp".  It copies from one filesystem (or a snapshot
> > of a filesystem) to another filesystem.
>
> Exactly.
>
> > Without multiple working directories, a modifiable "stash", or a
> > (useful) name for the filesystem referred to as
> > "index"/"cache"/"staging area", there is only one filesystem that the
> > command can write to: the (singular) working directory.
>
> No there are two writable "filesystems": working directory and
> "index/cache/staging area"
>
> > So, "git put <src filesystem> -- <path>" is fine.  It will copy from
> > the path in the src filesystem to the path in the current working
> > directory.  I don't think the command "put" is a great name for that.
> > Since we already have some strange double-usage commands like "git
> > checkout --" and "git reset --", perhaps this should be "git
> > cherry-pick --".
>
> The "-- <path>" thing may save you a few keystrokes when you want to
> copy from more than one path(spec). The two below commands are
> equivalent
>
> git put HEAD:a/ HEAD/b/ HEAD/c/ .
> git put HEAD: . -- a/ b/ c/
>
> But of course if you just need to copy from one pathspec to another
> place, "--" syntax is redundant.
>
> > <rant>
> > But for my money, "git cp" is clearer and I'd love to get rid of the
> > user-confusing double-usage commands.  I'd replace "git checkout --"
> > with "git cp NEXT WTREE -- <path>" and replace "git reset --" with
> > "git cp HEAD NEXT --" where NEXT is the filesystem represented by the
> > "index"/"cache"/"staging area" and WTREE is an alias for the working
> > directory.
> > </rant>
>
> I thought of "cp" (naturally, I was driven by "scp" syntax as I said)
> and maybe if we think this through, we may be able to enhance cp to
> support "remote locations" (and --patch option). So "put" vs "cp" is
> not important to me now. What I'd like to hear is whether the syntax
> makes sense.
>
> My "hidden" plan if this works out would be to deprecate (or
> discourage) everything in git-checkout except branch switching. I
> don't have anything against git-reset. It's a kind of dangerous
> command from the start (while git-checkout is more user friendly) and
> can stay that way. The new "git <cp, put or whatever name>" should
> fill 90% the needs for git-reset.
> --
> Duy

^ permalink raw reply

* [PATCH/RFC] Fix the result of "git grep -l -C <num>"
From: Albert Yale @ 2012-01-23 16:01 UTC (permalink / raw)
  To: git; +Cc: trast, rene.scharfe, Albert Yale

When combining "git grep -l" with "-C <num>",
the first result is omitted.

Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
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

The first result (a.txt) will be missing.

Understandably, you wouldn't normally use "-C" with "-l",
but the output should still be correct.

My solution is to take "opt.name_only" into account before setting
"skip_first_line" in grep.c.

I've reproduced this bug with git version 1.7.8.3
and git version 1.7.9.rc2, both under Mac OS X 10.7.2.

Albert Yale

 builtin/grep.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..076de21 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 )
+				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: 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox