Git development
 help / color / mirror / Atom feed
* {Spam?} push pull not working
From: Rick Bragg @ 2012-01-24 19:00 UTC (permalink / raw)
  To: GIT Mailing-list

Hi,

I cloned a repo from /home/me/repo1 to /home/me/repo2.  Then made
changes and a new commit on repo1, then from repo1 did "git
push /home/me/repo2 and it says Everything is up-to-date.  How could
this be?

Thanks
Rick

^ permalink raw reply

* [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Jehan Bing @ 2012-01-24 19:11 UTC (permalink / raw)
  To: git

Hi,

I'm getting an error if I try to add a module in a subdirectory and that 
module is already cloned.
Here are the steps to reproduce (git 1.7.8.3):

git init module
cd module
echo foo > foo
git add foo
git commit -m "init"
cd ..
git init super
cd super
echo foo > foo
git add foo
git commit -m "init"
git branch b1
git branch b2
git checkout b1
git submodule add ../module lib/module
git commit -m "module"
git checkout b2
rm -rf lib
git submodule add ../module lib/module

The last command returns:
     fatal: Not a git repository: ../.git/modules/lib/module
     Unable to checkout submodule 'lib/module'

The file lib/modules/.git contains:
     gitdir: ../.git/modules/lib/module
(missing an additional "../")

In branch b1, after adding the module, the file contained the full path:
     gitdir: /[...]/super/.git/modules/lib/module
Or contains the correct relative path after checking out b1 later:
     gitdir: ../../.git/modules/lib/module


Regards,
	Jehan

^ permalink raw reply

* Re: {Spam?} push pull not working
From: Jeff King @ 2012-01-24 19:41 UTC (permalink / raw)
  To: Rick Bragg; +Cc: GIT Mailing-list
In-Reply-To: <1327431631.21582.209.camel@thor>

On Tue, Jan 24, 2012 at 02:00:31PM -0500, Rick Bragg wrote:

> I cloned a repo from /home/me/repo1 to /home/me/repo2.  Then made
> changes and a new commit on repo1, then from repo1 did "git
> push /home/me/repo2 and it says Everything is up-to-date.  How could
> this be?

It's hard to say, since you didn't show us the exact commands you ran.

One possible cause is that you made your commit on a detached HEAD, not
on a branch, and therefore pushing branches will have no effect. You can
check this by running "git status", which will report either your
current branch or "not currently on any branch".

Another possible cause is that git is not trying to push the branches
that you think it is.

For example, imagine repo1 has two branches, "master" and "foo", and the
"master" branch is checked out. When you clone it, the resulting repo2
will have remote-tracking branches for both "master" and "foo", but will
only checkout the "master" branch. Now imagine you make commits on
"foo" in repo1, and then try to push. Git's default behavior is to push
only branches which match (by name) a branch on the destination. So we
would attempt to push "master" (which is up to date), but not "foo".

You can see which branches are being considered in the push with "git
push -vv". If you want to push all branches, you can use "git push
--all", or read up on refspecs in "git help push". If you want to change
git-push's default behavior, read up on "push.default" in "git help
config".

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Alex Riesen @ 2012-01-24 19:59 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Git Mailing List, Ævar Arnfjörð
In-Reply-To: <20120123221256.GG20833@burratino>

On Mon, Jan 23, 2012 at 23:12, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Here's a patch to consider squashing in that makes the option take
> effect if it changes between builds.

Which actually bit me once, when I was playing with the code :)

^ permalink raw reply

* Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Alex Riesen @ 2012-01-24 20:00 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Ævar Arnfjörð, Jonathan Nieder
In-Reply-To: <7vr4yq6poy.fsf_-_@alter.siamese.dyndns.org>

On Mon, Jan 23, 2012 at 23:04, Junio C Hamano <gitster@pobox.com> wrote:
> 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>

Amen :)

^ permalink raw reply

* {Spam?} Re: push pull not working
From: Rick Bragg @ 2012-01-24 20:05 UTC (permalink / raw)
  To: Jeff King; +Cc: GIT Mailing-list
In-Reply-To: <20120124194153.GB19534@sigill.intra.peff.net>

On Tue, 2012-01-24 at 14:41 -0500, Jeff King wrote:
> On Tue, Jan 24, 2012 at 02:00:31PM -0500, Rick Bragg wrote:
> 
> > I cloned a repo from /home/me/repo1 to /home/me/repo2.  Then made
> > changes and a new commit on repo1, then from repo1 did "git
> > push /home/me/repo2 and it says Everything is up-to-date.  How could
> > this be?
> 
> It's hard to say, since you didn't show us the exact commands you ran.
> 
> One possible cause is that you made your commit on a detached HEAD, not
> on a branch, and therefore pushing branches will have no effect. You can
> check this by running "git status", which will report either your
> current branch or "not currently on any branch".
> 
> Another possible cause is that git is not trying to push the branches
> that you think it is.
> 
> For example, imagine repo1 has two branches, "master" and "foo", and the
> "master" branch is checked out. When you clone it, the resulting repo2
> will have remote-tracking branches for both "master" and "foo", but will
> only checkout the "master" branch. Now imagine you make commits on
> "foo" in repo1, and then try to push. Git's default behavior is to push
> only branches which match (by name) a branch on the destination. So we
> would attempt to push "master" (which is up to date), but not "foo".
> 
> You can see which branches are being considered in the push with "git
> push -vv". If you want to push all branches, you can use "git push
> --all", or read up on refspecs in "git help push". If you want to change
> git-push's default behavior, read up on "push.default" in "git help
> config".
> 
> -Peff
> 

Thanks, I went back a few commits and tried again and it works.  I'm not
sure what it was, I will have allot of reading to do.

Thanks again!
Rick

^ permalink raw reply

* Re: [PATCH/RFC 3/2] i18n: do not use gettext.sh by default when NO_GETTEXT is set
From: Alex Riesen @ 2012-01-24 20:06 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Git Mailing List, Ævar Arnfjörð
In-Reply-To: <20120124003109.GB26980@burratino>

On Tue, Jan 24, 2012 at 01:31, Jonathan Nieder <jrnieder@gmail.com> wrote:
> +       ifndef USE_GETTEXT_SCHEME
> +               USE_GETTEXT_SCHEME = fallthrough
> +       endif

We already use GNU make features (+=), so this can be just

USE_GETTEXT_SCHEME ?= fallthrough

I think...

^ permalink raw reply

* Re: {Spam?} push pull not working
From: Junio C Hamano @ 2012-01-24 20:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Rick Bragg, GIT Mailing-list
In-Reply-To: <20120124194153.GB19534@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> For example, imagine repo1 has two branches, "master" and "foo", and the
> "master" branch is checked out. When you clone it, the resulting repo2
> will have remote-tracking branches for both "master" and "foo", but will
> only checkout the "master" branch. Now imagine you make commits on
> "foo" in repo1, and then try to push. Git's default behavior is to push
> only branches which match (by name) a branch on the destination. So we
> would attempt to push "master" (which is up to date), but not "foo".

Technically you are not saying anything incorrect, but the above is not an
appropriate paragraph to give to a total newbie, I would have to say.

It does not make it clear that it is insane to push to repo2 with matching
push (or "current" or "upstream" for that matter), after cloning from a
repository repo1 with a working tree to repo2 with an working tree, and
making changes at the original repo1 repository. Instead, you are giving a
false impression that pushing both "master" and "foo" explicitly would
solve the problem OP is having, which is not true.

If repo1 and repo2 with working trees want to criss-cross-exchange their
histories, both should "git pull" from the other, not "git push", and if
one side cannot initiate a connection to the other, "git push" that goes
in the other direction should be pushing into remote tracking refs of the
other in order to emulate a "git pull" that goes in the other direction.

^ permalink raw reply

* Re: [PATCH 2/2] add a Makefile switch to avoid gettext translation in shell scripts
From: Junio C Hamano @ 2012-01-24 20:13 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Git Mailing List, Ævar Arnfjörð, Jonathan Nieder
In-Reply-To: <CALxABCaGMabTLcCiYLv31YCiVY4OK7yEr4KL6e-0UMttMjGA_g@mail.gmail.com>

> Amen :)

Thanks.

^ permalink raw reply

* Re: {Spam?} push pull not working
From: Jeff King @ 2012-01-24 20:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Rick Bragg, GIT Mailing-list
In-Reply-To: <7vty3k3lmh.fsf@alter.siamese.dyndns.org>

On Tue, Jan 24, 2012 at 12:12:54PM -0800, Junio C Hamano wrote:

> > For example, imagine repo1 has two branches, "master" and "foo", and the
> > "master" branch is checked out. When you clone it, the resulting repo2
> > will have remote-tracking branches for both "master" and "foo", but will
> > only checkout the "master" branch. Now imagine you make commits on
> > "foo" in repo1, and then try to push. Git's default behavior is to push
> > only branches which match (by name) a branch on the destination. So we
> > would attempt to push "master" (which is up to date), but not "foo".
> 
> Technically you are not saying anything incorrect, but the above is not an
> appropriate paragraph to give to a total newbie, I would have to say.
> [...]

Yeah, I agree with all of this. I was trying not to go into too much
detail because we knew so little about the situation (e.g., we don't
even know if repo2 in the example is bare or not!), but perhaps my
terseness made things even more confusing.

This might have been a better example (it exhibits the problem, but is
not an example of a terrible thing to be doing):

  1. repo1 has a "master" branch

  2. clone repo1 with "git clone --bare repo1 repo2". Repo2 now has a
     master branch.

  3. create a new "foo" branch in repo and commit on it

  4. "git push ../repo2" from repo1. This is a sane thing to be doing,
     but will not push the newly-created "foo" branch, as some users
     might expect.

-Peff

^ permalink raw reply

* Re: [BUG] Git bisect not finding the right commit
From: Jeff King @ 2012-01-24 20:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Andreas J. Koenig
In-Reply-To: <7vk44mfdow.fsf@alter.siamese.dyndns.org>

On Fri, Jan 20, 2012 at 10:09:51AM -0800, Junio C Hamano wrote:

> > Yes, thank you for finding that out. X is actually v5.15.4-109-g3ea0c58
> > and since there was a long timespan between the start of the development
> > of the code and the merge (May-Nov), the gitk presentation got a bit
> > complex to read.
> 
> (This is somewhat off-topic, so Andreas is on Cc: and the list is on To:)
> 
> I doubt --simplify-by-decoration alone would make it easier to view such a
> complex and long history, but I wonder if we can use the same logic to
> help users in a case like this. Instead of only keeping tagged versions in
> the result to show topology, perhaps we can allow the user to feed a list
> of "key points in history" as command line arguments and apply the same
> kind of simplification to help visualizing the topology?

This is something I think I've wanted in the past. But unfortunately, I
can't remember the exact details, so I'm not sure how workable it would
have been. In particular, how painful is it in practice to come up with
the list of "key points" to make the graph sensible?

So I'll add my vague "yeah, that sounds good" and try to pay attention
next time the situation comes up as to whether such a feature would
actually help in practice.

-Peff

^ permalink raw reply

* Re: {Spam?} push pull not working
From: Junio C Hamano @ 2012-01-24 20:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Rick Bragg, GIT Mailing-list
In-Reply-To: <20120124201807.GA20145@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This might have been a better example (it exhibits the problem, but is
> not an example of a terrible thing to be doing):
>
>   1. repo1 has a "master" branch
>
>   2. clone repo1 with "git clone --bare repo1 repo2". Repo2 now has a
>      master branch.
>
>   3. create a new "foo" branch in repo and commit on it
>
>   4. "git push ../repo2" from repo1. This is a sane thing to be doing,
>      but will not push the newly-created "foo" branch, as some users
>      might expect.

Yeah, that is pretty much the standard thing people would do, at least
before GitHub era ;-), to start a project in repo1, and then to publish
for others to fetch at repo2.

Thanks for clarification.

P.S. Did you have chance to take a look at the "grep" thing? I thought
"grep --textconv" would make sense, but I may be missing some large
pitfalls.

^ permalink raw reply

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Jens Lehmann @ 2012-01-24 21:10 UTC (permalink / raw)
  To: Jehan Bing; +Cc: git
In-Reply-To: <jfmvpp$4v7$1@dough.gmane.org>

Am 24.01.2012 20:11, schrieb Jehan Bing:
> I'm getting an error if I try to add a module in a subdirectory and that module is already cloned.
> Here are the steps to reproduce (git 1.7.8.3):
> 
> git init module
> cd module
> echo foo > foo
> git add foo
> git commit -m "init"
> cd ..
> git init super
> cd super
> echo foo > foo
> git add foo
> git commit -m "init"
> git branch b1
> git branch b2
> git checkout b1
> git submodule add ../module lib/module
> git commit -m "module"
> git checkout b2
> rm -rf lib
> git submodule add ../module lib/module
> 
> The last command returns:
>     fatal: Not a git repository: ../.git/modules/lib/module
>     Unable to checkout submodule 'lib/module'
> 
> The file lib/modules/.git contains:
>     gitdir: ../.git/modules/lib/module
> (missing an additional "../")
> 
> In branch b1, after adding the module, the file contained the full path:
>     gitdir: /[...]/super/.git/modules/lib/module
> Or contains the correct relative path after checking out b1 later:
>     gitdir: ../../.git/modules/lib/module

Thanks for your detailed report, I can reproduce that on current master.

The reason for this bug seems to be that in module_clonse() the name is
not properly initialized for added submodules (it gets set to the path
later), so the correct amount of leading "../"s for the git directory
is not computed properly. The attached diff fixes that for me, I will
send a patch as soon as I have extended a test case for this breakage.

diff --git a/git-submodule.sh b/git-submodule.sh
index 3adab93..9bb2e13 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -131,6 +131,7 @@ module_clone()
        gitdir=
        gitdir_base=
        name=$(module_name "$path" 2>/dev/null)
+       test -n "$name" || name="$path"
        base_path=$(dirname "$path")

        gitdir=$(git rev-parse --git-dir)

^ permalink raw reply related

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Jens Lehmann @ 2012-01-24 21:13 UTC (permalink / raw)
  To: Jehan Bing; +Cc: git
In-Reply-To: <4F1F1E5F.2030509@web.de>

Am 24.01.2012 22:10, schrieb Jens Lehmann:
> The reason for this bug seems to be that in module_clonse() the name is
> not properly initialized for added submodules ...

This should have read "for re-added submodules" ...

^ permalink raw reply

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Junio C Hamano @ 2012-01-24 21:24 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Jehan Bing, git
In-Reply-To: <4F1F1E5F.2030509@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> The reason for this bug seems to be that in module_clonse() the name is
> not properly initialized for added submodules (it gets set to the path
> later), so the correct amount of leading "../"s for the git directory
> is not computed properly. The attached diff fixes that for me, I will
> send a patch as soon as I have extended a test case for this breakage.
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3adab93..9bb2e13 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -131,6 +131,7 @@ module_clone()
>         gitdir=
>         gitdir_base=
>         name=$(module_name "$path" 2>/dev/null)
> +       test -n "$name" || name="$path"

This somehow smells like sweeping a problem under the rug. Why doesn't
module_name find the already registered path in the first place?

I see "module_name" calls "git config -f .gitmodules" and I do not see any
cd_to_toplevel in git-submodule.sh that would ensure this call to access
the gitmodules file at the top-level of the superproject. Is that the real
reason why it is not finding what it should be finding?

^ permalink raw reply

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Jens Lehmann @ 2012-01-24 21:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jehan Bing, git
In-Reply-To: <7vhazk3ibk.fsf@alter.siamese.dyndns.org>

Am 24.01.2012 22:24, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> The reason for this bug seems to be that in module_clonse() the name is
>> not properly initialized for added submodules (it gets set to the path
>> later), so the correct amount of leading "../"s for the git directory
>> is not computed properly. The attached diff fixes that for me, I will
>> send a patch as soon as I have extended a test case for this breakage.
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 3adab93..9bb2e13 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -131,6 +131,7 @@ module_clone()
>>         gitdir=
>>         gitdir_base=
>>         name=$(module_name "$path" 2>/dev/null)
>> +       test -n "$name" || name="$path"
> 
> This somehow smells like sweeping a problem under the rug. Why doesn't
> module_name find the already registered path in the first place?
> 
> I see "module_name" calls "git config -f .gitmodules" and I do not see any
> cd_to_toplevel in git-submodule.sh that would ensure this call to access
> the gitmodules file at the top-level of the superproject. Is that the real
> reason why it is not finding what it should be finding?

Nope, it's the fact that the .gitmodules file doesn't contain this name
because the branch was rewound. Please see my post where I proposed the
same change for a slightly different problem:
  http://permalink.gmane.org/gmane.comp.version-control.git/187823
(just fast forward to the first hunk of my diff at the end)

I just didn't realize back then that this is needed even without the
other changes to work properly. The possibly missing cd_to_toplevel is
another problem, the OP started the submodule add in the top level
directory anyways.

^ permalink raw reply

* [PATCH] submodule add: fix breakage when re-adding a deep submodule
From: Jens Lehmann @ 2012-01-24 21:49 UTC (permalink / raw)
  To: Jehan Bing; +Cc: git, Junio C Hamano
In-Reply-To: <4F1F1E5F.2030509@web.de>

Since recently a submodule with name <name> has its git directory in the
.git/modules/<name> directory of the superproject while the work tree
contains a gitfile pointing there.

When the same submodule is added on a branch where it wasn't present so
far (it is not found in the .gitmodules file), the name is not initialized
from the path as it should. This leads to a wrong path entered in the
gitfile when the .git/modules/<name> directory is found, as this happily
uses the - now empty - name. It then always points only a single directory
up, even if we have a path deeper in the directory hierarchy.

Fix that by initializing the name of the submodule early in module_clone()
if module_name() returned an empty name and add a test to catch that bug.

Reported-by: Jehan Bing <jehan@orb.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 24.01.2012 22:10, schrieb Jens Lehmann:
> Am 24.01.2012 20:11, schrieb Jehan Bing:
>> I'm getting an error if I try to add a module in a subdirectory and that module is already cloned.
>> Here are the steps to reproduce (git 1.7.8.3):

...

> The reason for this bug seems to be that in module_clonse() the name is
> not properly initialized for added submodules (it gets set to the path
> later), so the correct amount of leading "../"s for the git directory
> is not computed properly. The attached diff fixes that for me, I will
> send a patch as soon as I have extended a test case for this breakage.

Which I now have.


 git-submodule.sh            |    1 +
 t/t7406-submodule-update.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3adab93..9bb2e13 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -131,6 +131,7 @@ module_clone()
 	gitdir=
 	gitdir_base=
 	name=$(module_name "$path" 2>/dev/null)
+	test -n "$name" || name="$path"
 	base_path=$(dirname "$path")

 	gitdir=$(git rev-parse --git-dir)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 33b292b..5b97222 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -611,4 +611,12 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
 	)
 '

+test_expect_success 'submodule add properly re-creates deeper level submodules' '
+	(cd super &&
+	 git reset --hard master &&
+	 rm -rf deeper/ &&
+	 git submodule add ../submodule deeper/submodule
+	)
+'
+
 test_done
-- 
1.7.9.rc2.3.g18574a

^ permalink raw reply related

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Jens Lehmann @ 2012-01-24 22:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <4F1F2642.1070707@web.de>

Am 24.01.2012 22:44, schrieb Jens Lehmann:
> Am 24.01.2012 22:24, schrieb Junio C Hamano:
>> I see "module_name" calls "git config -f .gitmodules" and I do not see any
>> cd_to_toplevel in git-submodule.sh that would ensure this call to access
>> the gitmodules file at the top-level of the superproject. Is that the real
>> reason why it is not finding what it should be finding?

Just for the record: I checked that and git-submodule does not set the
SUBDIRECTORY_OK environment variable so every time it is not run in the
top level directory it aborts with:
"You need to run this command from the toplevel of the working tree."

^ permalink raw reply

* [PATCH 1/5] t0061: Fix incorrect indentation
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt, Frans Klaver
In-Reply-To: <1327444346-6243-1-git-send-email-fransklaver@gmail.com>

The hello.sh script started with <tab>#!, which is not considered a
correct hash-bang line. The default interpreter happened to be the one
required for this specific test.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 t/t0061-run-command.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 8d4938f..95e89bc 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -8,8 +8,8 @@ test_description='Test run command'
 . ./test-lib.sh
 
 cat >hello-script <<-EOF
-	#!$SHELL_PATH
-	cat hello-script
+#!$SHELL_PATH
+cat hello-script
 EOF
 >empty
 
-- 
1.7.8.1

^ permalink raw reply related

* [PATCH 3/5] run-command: Elaborate execvp error checking
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt, Frans Klaver
In-Reply-To: <1327444346-6243-1-git-send-email-fransklaver@gmail.com>

The interpretation of errors from execvp was rather terse. For user
convenience communication of the nature of the error can be improved.

This patch introduces a function with more elaborate investigation of
the errors encountered. After a failure occurs on a program or script,
inspect_failure will run through the PATH entries if necessary, and if
the file exists, perform some tests on the file. If nothing is found,
ENOENT is returned.

The function inspect_file() will try to find out if the file itself or
an interpreter caused the issue. Some scripts don't have the hash-bang
and will not be reported as scripts. If the interpreter cannot be found,
the existing behavior is kept -- i.e. ENOENT will be reported to the
parent process.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |  144 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t0061-run-command.sh |    6 +-
 2 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/run-command.c b/run-command.c
index 1c51043..17a65fe 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {
@@ -134,6 +135,140 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int is_executable_file(const char *path)
+{
+	return access(path, X_OK) == 0 && !is_directory(path);
+}
+
+static int is_searchable(const char *path)
+{
+	return access(path, X_OK) == 0 && is_directory(path);
+}
+
+static void die_file_error(const char *file, int err)
+{
+	die("cannot exec '%s': %s", file, strerror(err));
+}
+
+static char *get_interpreter(const char *first_line)
+{
+	struct strbuf sb = STRBUF_INIT;
+	size_t start = strspn(first_line + 2, " \t") + 2;
+	size_t end = strcspn(first_line + start, " \t\r\n") + start;
+
+	if (start >= end)
+		return NULL;
+
+	strbuf_add(&sb, first_line + start, end - start);
+	return strbuf_detach(&sb, NULL);
+}
+
+static void inspect_file(struct strbuf *fn, int err, const char *argv0)
+{
+	/*
+	 * Typical line length of 80. BSD only allows 32 bytes, but that
+	 * won't really make a difference.
+	 */
+	char buf[80];
+	size_t read;
+	FILE *file;
+	if (!is_executable_file(fn->buf)) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	file = fopen(fn->buf, "r");
+	if (!file) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	read = fread(buf, 1, sizeof(buf), file);
+	fclose(file);
+	if (read > 2 && buf[0] == '#' && buf[1] == '!') {
+		char *i = get_interpreter(buf);
+		char es[1024];
+		if (i) {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter '%s': %s",
+					argv0, i,
+					strerror(err));
+		} else {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter: %s",
+					argv0, strerror(err));
+		}
+		free(i);
+		strbuf_release(fn);
+		if (err == ENOENT) {
+			error("%s", es);
+			exit(127);
+		} else {
+			die("%s", es);
+		}
+	}
+
+	strbuf_release(fn);
+	die_file_error(argv0, err);
+}
+
+static void inspect_failure(const char *argv0, int silent_exec_failure)
+{
+	int err = errno;
+	struct strbuf sb = STRBUF_INIT;
+
+	/* errors not related to path */
+	if (errno == E2BIG || errno == ENOMEM)
+		die_file_error(argv0, err);
+
+	if (strchr(argv0, '/')) {
+		if (file_exists(argv0)) {
+			strbuf_add(&sb, argv0, strlen(argv0));
+			inspect_file(&sb, err, argv0);
+		}
+	} else {
+		char *path, *next;
+		path = getenv("PATH");
+		while (path) {
+			next = strchrnul(path, ':');
+			if (path < next)
+				strbuf_add(&sb, path, next - path);
+			else
+				strbuf_addch(&sb, '.');
+
+			if (!*next)
+				path = NULL;
+			else
+				path = next + 1;
+
+			if (!is_searchable(sb.buf)) {
+				strbuf_release(&sb);
+				continue;
+			}
+
+			if (sb.len && sb.buf[sb.len - 1] != '/')
+				strbuf_addch(&sb, '/');
+			strbuf_addstr(&sb, argv0);
+
+			if (file_exists(sb.buf))
+				inspect_file(&sb, err, argv0);
+
+			strbuf_release(&sb);
+		}
+	}
+
+	if (err == ENOENT) {
+		if (!silent_exec_failure)
+			error("cannot exec '%s': %s", argv0,
+					strerror(ENOENT));
+		exit(127);
+	} else {
+		die_file_error(argv0, err);
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
@@ -280,14 +415,7 @@ fail_pipe:
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
-		if (errno == ENOENT) {
-			if (!cmd->silent_exec_failure)
-				error("cannot run %s: %s", cmd->argv[0],
-					strerror(ENOENT));
-			exit(127);
-		} else {
-			die_errno("cannot exec '%s'", cmd->argv[0]);
-		}
+		inspect_failure(cmd->argv[0], cmd->silent_exec_failure);
 	}
 	if (cmd->pid < 0)
 		error("cannot fork() for %s: %s", cmd->argv[0],
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 31eb3c3..14b4ea6 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -71,7 +71,8 @@ test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
 	chmod -x someinterpreter &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&
 
-	grep "fatal: cannot exec.*hello.sh" err
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
@@ -79,7 +80,8 @@ test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
 	chmod +x hello.sh &&
 	test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err &&
 
-	grep "error: cannot exec.*hello.sh" err
+	grep "error: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_done
-- 
1.7.8.1

^ permalink raw reply related

* [PATCH 0/6 v3] Add execvp failure diagnostics
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt

This patch series replaces $gmane/187026. It aims to improve the
information provided after execvp fails in run-command.

This series takes a rather different approach than the previous
one. In the former the focus was on determining what might cause
an EACCES failure. This series focuses on trying to provide hints
on what went wrong on any error.

The resulting checking produces behavior rather like bash, with the
notable exception that we consider "Not found and PATH access issues"
a failure, where bash doesn't.

[PATCH 1/5] t0061: Fix incorrect indentation
[PATCH 2/5] t0061: Add tests
[PATCH 3/5] run-command: Elaborate execvp error checking
[PATCH 4/5] run-command: Warn if PATH entry cannot be searched
[PATCH 5/5] run-command: Error out if interpreter not found

run-command.c          |  140 +++++++++++++++++++++++++++++++++++++++++++++---
t/t0061-run-command.sh |   57 ++++++++++++++++++-
2 files changed, 186 insertions(+), 11 deletions(-)

^ permalink raw reply

* [PATCH 4/5] run-command: Warn if PATH entry cannot be searched
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt, Frans Klaver
In-Reply-To: <1327444346-6243-1-git-send-email-fransklaver@gmail.com>

execvp will return EACCES when PATH entries are encountered that cannot
be searched and the requested command isn't found. Since this error only
arises in that particular case, warning the user is more appropriate
than producing an error.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |    1 +
 t/t0061-run-command.sh |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/run-command.c b/run-command.c
index 17a65fe..ab14910 100644
--- a/run-command.c
+++ b/run-command.c
@@ -243,6 +243,7 @@ static void inspect_failure(const char *argv0, int silent_exec_failure)
 				path = next + 1;
 
 			if (!is_searchable(sb.buf)) {
+				warning("cannot search '%s'", sb.buf);
 				strbuf_release(&sb);
 				continue;
 			}
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 14b4ea6..a4585b0 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -62,7 +62,8 @@ test_expect_success POSIXPERM 'run_command reports EACCES, search path perms' '
 	test_must_fail test-run-command run-command hello.sh 2>err &&
 	chmod 755 inaccessible &&
 
-	grep "fatal: cannot exec.*hello.sh" err
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "cannot search" err
 '
 
 test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
-- 
1.7.8.1

^ permalink raw reply related

* [PATCH 5/5] run-command: Error out if interpreter not found
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt, Frans Klaver
In-Reply-To: <1327444346-6243-1-git-send-email-fransklaver@gmail.com>

If the interpreter wasn't found, execvp returns ENOENT. The existing
error checking did not differentiate between file not found and
interpreter not found. While the former may be an incentive to start
inspecting aliases, the latter is an error because the desired script is
actually found.

This patch explicitly makes the interpreter failure a fatal error.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
As far as I'm concerned, this is a bug fix. However, since it really is
a change in git's behavior, we can still consider postponing this patch.

 run-command.c          |    7 +------
 t/t0061-run-command.sh |    4 ++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/run-command.c b/run-command.c
index ab14910..9179daf 100644
--- a/run-command.c
+++ b/run-command.c
@@ -201,12 +201,7 @@ static void inspect_file(struct strbuf *fn, int err, const char *argv0)
 		}
 		free(i);
 		strbuf_release(fn);
-		if (err == ENOENT) {
-			error("%s", es);
-			exit(127);
-		} else {
-			die("%s", es);
-		}
+		die("%s", es);
 	}
 
 	strbuf_release(fn);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index a4585b0..f08163f 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -76,12 +76,12 @@ test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
 	grep "bad interpreter" err
 '
 
-test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
+test_expect_success POSIXPERM 'run_command reports ENOENT, interpreter' '
 	cat non-existing-interpreter >hello.sh &&
 	chmod +x hello.sh &&
 	test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err &&
 
-	grep "error: cannot exec.*hello.sh" err &&
+	grep "fatal: cannot exec.*hello.sh" err &&
 	grep "bad interpreter" err
 '
 
-- 
1.7.8.1

^ permalink raw reply related

* [PATCH 2/5] t0061: Add tests
From: Frans Klaver @ 2012-01-24 22:32 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano, Jonathan Nieder, Johannes Sixt, Frans Klaver
In-Reply-To: <1327444346-6243-1-git-send-email-fransklaver@gmail.com>

Capture failure behavior when running into
- EACCES caused by search path permissions
- ENOENT caused by interpreter not found

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 t/t0061-run-command.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 95e89bc..31eb3c3 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -13,6 +13,24 @@ cat hello-script
 EOF
 >empty
 
+cat >someinterpreter <<-EOF
+#!$SHELL_PATH
+cat hello-script
+EOF
+>empty
+
+cat >incorrect-interpreter-script <<-EOF
+#!someinterpreter
+cat hello-script
+EOF
+>empty
+
+cat >non-existing-interpreter <<-EOF
+#!nonexisting_interpreter
+cat hello-script
+EOF
+>empty
+
 test_expect_success 'start_command reports ENOENT' '
 	test-run-command start-command-ENOENT ./does-not-exist
 '
@@ -26,7 +44,7 @@ test_expect_success 'run_command can run a command' '
 	test_cmp empty err
 '
 
-test_expect_success POSIXPERM 'run_command reports EACCES' '
+test_expect_success POSIXPERM 'run_command reports EACCES, file permissions' '
 	cat hello-script >hello.sh &&
 	chmod -x hello.sh &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&
@@ -34,4 +52,34 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, search path perms' '
+	mkdir -p inaccessible &&
+	PATH=$(pwd)/inaccessible:$PATH &&
+	export PATH &&
+
+	cat hello-script >inaccessible/hello.sh &&
+	chmod 400 inaccessible &&
+	test_must_fail test-run-command run-command hello.sh 2>err &&
+	chmod 755 inaccessible &&
+
+	grep "fatal: cannot exec.*hello.sh" err
+'
+
+test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
+	cat incorrect-interpreter-script >hello.sh &&
+	chmod +x hello.sh &&
+	chmod -x someinterpreter &&
+	test_must_fail test-run-command run-command ./hello.sh 2>err &&
+
+	grep "fatal: cannot exec.*hello.sh" err
+'
+
+test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
+	cat non-existing-interpreter >hello.sh &&
+	chmod +x hello.sh &&
+	test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err &&
+
+	grep "error: cannot exec.*hello.sh" err
+'
+
 test_done
-- 
1.7.8.1

^ permalink raw reply related

* Re: [BUG] Fail to add a module in a subdirectory if module is already cloned
From: Junio C Hamano @ 2012-01-24 22:38 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git
In-Reply-To: <4F1F2D38.9050909@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Just for the record: I checked that and git-submodule does not set the
> SUBDIRECTORY_OK environment variable so every time it is not run in the
> top level directory it aborts with:
> "You need to run this command from the toplevel of the working tree."

Ah, Ok.

^ permalink raw reply


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