rc4 - make quick-install-doc is broken

rc4 - make quick-install-doc is broken

[Mark Levedahl]

Beginning with commit

6490a3383f1d0d96c122069e510ef1af1d019fbb Fix work-tree related breakages

install-doc-quick.sh no longer installs man pages, at least not to the 
defined $mandir (if "git-checkout-index" is putting them somewhere else, 
I haven't discovered where). Reverting the above commit eliminates this 
problem. This is on Cygwin, haven't tried on Linux.

Mark Levedahl

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Mark Levedahl wrote:

> Beginning with commit
> 
> 6490a3383f1d0d96c122069e510ef1af1d019fbb Fix work-tree related breakages
> 
> install-doc-quick.sh no longer installs man pages, at least not to the defined
> $mandir (if "git-checkout-index" is putting them somewhere else, I haven't
> discovered where). Reverting the above commit eliminates this problem. This is
> on Cygwin, haven't tried on Linux.

Could it be that you did not find this commit by bisecting the issue?  I 
highly doubt that said commit changes anything in the build process.

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

Johannes Schindelin wrote:
> Could it be that you did not find this commit by bisecting the issue?  I 
> highly doubt that said commit changes anything in the build process.
>
> Ciao,
> Dscho
>
>   
<snip ...>

git>git bisect good
6490a3383f1d0d96c122069e510ef1af1d019fbb is first bad commit
commit 6490a3383f1d0d96c122069e510ef1af1d019fbb
Author: Junio C Hamano <gitster@pobox.com>
Date:   Thu Aug 2 15:10:56 2007 -0700

    Fix work-tree related breakages

    In set_work_tree(), variable rel needs to be reinitialized to
    NULL on every call (it should not be static).

    Make sure the incoming dir variable is not too long before
    copying to the temporary buffer, and make sure chdir to the
    resulting directory succeeds.

    This was spotted and fixed by Alex and Johannes in a handful
    patch exchanges.  Here is the final version.

    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

:100644 100644 3653092ab657942639b6183fb04b3af783b77e7a 
4945eb3134c3e047f54e51db25cd0aa81d9c47d7 M      setup.c

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Mark Levedahl wrote:

> git>git bisect good

Could you please

	GIT_TRACE=1 make quick-install-doc

? It breaks here, too, but because I have no origin/man branch.

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

Johannes Schindelin wrote:
>
> Could you please
>
> 	GIT_TRACE=1 make quick-install-doc
>
> ? It breaks here, too, but because I have no origin/man branch.
>
> Ciao,
> Dscho
>
>
>   
git>GIT_TRACE=1 make prefix=/usr quick-install-doc
make -C Documentation quick-install
make[1]: Entering directory `/usr/src/git/Documentation'
make -C ../ GIT-VERSION-FILE
make[2]: Entering directory `/usr/src/git'
make[2]: `GIT-VERSION-FILE' is up to date.
make[2]: Leaving directory `/usr/src/git'
sh ./install-doc-quick.sh origin/man /usr/share/man
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--verify' 'origin/man^0'
trace: built-in: git 'read-tree' 'origin/man'
trace: built-in: git 'checkout-index' '-a' '-f' '--prefix=/usr/share/man/'
make[1]: Leaving directory `/usr/src/git/Documentation'
git>man git
No manual entry for git
git>

Mark

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Mark Levedahl wrote:

> Johannes Schindelin wrote:
> > 
> > Could you please
> > 
> > 	GIT_TRACE=1 make quick-install-doc
> > 
> > ? It breaks here, too, but because I have no origin/man branch.
> > 
> > Ciao,
> > Dscho
> > 
> > 
> >   
> git>GIT_TRACE=1 make prefix=/usr quick-install-doc
> make -C Documentation quick-install
> make[1]: Entering directory `/usr/src/git/Documentation'
> make -C ../ GIT-VERSION-FILE
> make[2]: Entering directory `/usr/src/git'
> make[2]: `GIT-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/usr/src/git'
> sh ./install-doc-quick.sh origin/man /usr/share/man
> trace: built-in: git 'rev-parse' '--git-dir'
> trace: built-in: git 'rev-parse' '--verify' 'origin/man^0'
> trace: built-in: git 'read-tree' 'origin/man'
> trace: built-in: git 'checkout-index' '-a' '-f' '--prefix=/usr/share/man/'
> make[1]: Leaving directory `/usr/src/git/Documentation'

At this point, could you try debugging it?  For example, set a break point 
in set_work_tree(), since that is the only function that commit touches, 
and see what it returns both before and after the bad commit?

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

Johannes Schindelin wrote:
> At this point, could you try debugging it?  For example, set a break point 
> in set_work_tree(), since that is the only function that commit touches, 
> and see what it returns both before and after the bad commit?
>
> Ciao,
> Dscho
>
>
>   
Before the bad commit, set_work_tree returns (null), and
variable dir_buffer in the call to get_relative_cwd is 
"/usr/src/git/.git/HEAD"

After, it returns "Documentation/", and
variable dir_buffer in the call to get_relative_cwd is "/usr/src/git"

I do not understand what the desired changes in this patch were so am 
really lost as to what should be fixed. However, the above I hope will 
give you a good clue as to what is broken.

Mark

[PATCH] Fix quick-install-doc

[Johannes Schindelin]

The recent work-tree cleanups exposed that the install-doc-quick
script was relying on a strange behaviour predating the work-tree
series: when setting a GIT_DIR, it was assumed that the current
working directory is the root of the working tree.

The recent work-tree series changed that behaviour: now that you can
set the work tree explicitely, the work tree root defaults to
$GIT_DIR/.. if $GIT_DIR has a /.git suffix when that is a parent
directory of the current working directory.

Noticed by Mark Levedahl. Diagnosed by Junio Hamano.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Junio, I did it slightly differently from what I said on IRC: 
	setting the work-tree with "--work-tree" does not work as 
	expected.  Will fix.

 Documentation/install-doc-quick.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/install-doc-quick.sh b/Documentation/install-doc-quick.sh
index e6601bd..5c8b604 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -19,7 +19,7 @@ GIT_INDEX_FILE=`pwd`/.quick-doc.index
 export GIT_INDEX_FILE
 rm -f "$GIT_INDEX_FILE"
 git read-tree $head
-git checkout-index -a -f --prefix="$mandir"/
+(cd "$mandir"; git checkout-index -a -f)
 
 if test -n "$GZ"; then
 	cd "$mandir"
-- 
1.5.3.rc4.1.g7805

Re: [PATCH] Fix quick-install-doc

[René Scharfe]

Johannes Schindelin schrieb:
> The recent work-tree cleanups exposed that the install-doc-quick
> script was relying on a strange behaviour predating the work-tree
> series: when setting a GIT_DIR, it was assumed that the current
> working directory is the root of the working tree.
> 
> The recent work-tree series changed that behaviour: now that you can
> set the work tree explicitely, the work tree root defaults to
> $GIT_DIR/.. if $GIT_DIR has a /.git suffix when that is a parent
> directory of the current working directory.

> -git checkout-index -a -f --prefix="$mandir"/
> +(cd "$mandir"; git checkout-index -a -f)

That won't work if $mandir doesn't exist, which can happen if you
install the manpages for the first time.  Simply add a mkdir:

   (mkdir -p "$mandir" && cd "$mandir" && git checkout-index -a -f)

René

[PATCH] Fix install-doc-quick target

[Junio C Hamano]

The script starts in a subdirectory of the source directory to
muck with a branch whose structure does not have anything to
do with the actual work tree.  Go up to the top to make it clear
that we operate on the whole tree.

It also exported GIT_DIR without any good reason.  Remove it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I am getting the impression that the semantics of the updated
   work-tree support is as broken as the original, although the
   code that implements it is easier to read.  

   Admittedly, this is an unorthodox corner case usage, that
   deals with a tree structure that does not have any relation
   with the current project state, and the script is started
   with a subdirectory of the current project.  What is needed
   is a way to tell git that "we may be in a subdirectory but
   the tree structure we are dealing with does not have anything
   to do with the current work tree structure -- ignore the fact
   that we are not at the toplevel", and exporting of GIT_DIR
   was the way to do so in the old world order.

   Another way to do this in the new world order would be to
   pass "--work-tree=.", but this has its own problems it
   seems.  You can try commenting out cd_to_toplevel this patch
   introduces, then rewrite checkout-index part with:

    git --git-dir="$GIT_DIR" --work-tree=. checkout-index -a...

   which seems to work, but if you drop --git-dir="$GIT_DIR"
   from the above (without reintroducing the export of GIT_DIR),
   you get "fatal: Could not switch to '.git'" which seems quite
   bogus.  If you say:

	git --work-tree=. foo

   without saying anything about GIT_DIR, shouldn't we run the
   usual .git/ discovery, going up the directories? 

 Documentation/install-doc-quick.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Documentation/install-doc-quick.sh b/Documentation/install-doc-quick.sh
index e6601bd..07d227f 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -7,7 +7,7 @@ mandir="$2"
 SUBDIRECTORY_OK=t
 USAGE='<refname> <target directory>'
 . git-sh-setup
-export GIT_DIR
+cd_to_toplevel
 
 test -z "$mandir" && usage
 if ! git rev-parse --verify "$head^0" >/dev/null; then
@@ -18,6 +18,8 @@ fi
 GIT_INDEX_FILE=`pwd`/.quick-doc.index
 export GIT_INDEX_FILE
 rm -f "$GIT_INDEX_FILE"
+trap 'rm -f "$GIT_INDEX_FILE"' 0
+
 git read-tree $head
 git checkout-index -a -f --prefix="$mandir"/
 
-- 
1.5.3.rc4.8.ga120

Re: [PATCH] Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Sun, 5 Aug 2007, Junio C Hamano wrote:

>  * I am getting the impression that the semantics of the updated
>    work-tree support is as broken as the original, although the
>    code that implements it is easier to read.  
> 
>    Admittedly, this is an unorthodox corner case usage, that
>    deals with a tree structure that does not have any relation
>    with the current project state, and the script is started
>    with a subdirectory of the current project.

I do not like the behaviour "be stupid and assume cwd to be the working 
tree root, if GIT_DIR is set and GIT_WORK_TREE is not".

It bears _all_ kind of stupid connotations.  Just imagine what would 
happen with "git --git-dir=. add .".

IMHO the new behaviour is _better_, since you can not shoot yourself in 
the foot so easily.  Being able to safeguard against doing a work tree 
operation inside the git directory is a direct and elegant consequence of 
defaulting to $GIT_DIR/.. in case $GIT_DIR ends in "/.git", and no work 
tree if $GIT_DIR does _not_ end in "/.git".

The semantics "if GIT_DIR is set, just assume the cwd to be the work tree 
root unilaterally" is _broken_ as far as I am concerned.

NOTE: this change in semantics will _only_ _ever_ burn you if you set 
GIT_DIR, but not GIT_WORK_TREE, in a _subdirectory_ of $GIT_DIR/.. (if it 
ends in /.git, and $GIT_DIR otherwise).

So the _common_ use of setting GIT_DIR, namely something like

	$ git --git-dir=/some/where/completely/different bla

is _not_ affected.

So I really think that the code in install-doc-quick.sh is not only ugly, 
but also wrong.  It sets the GIT_DIR to _the same_ value as the default, 
to change git's idea of the _work tree_!  All at the same time as it is 
utterly clear that it wants to write to $HOME/share/man, but it does not 
make this its working tree.  No, sir.  It has to play cute games with the 
prefix.

However, you are the maintainer, and it is your decision.  If you want the 
old semantics (without sanity checks for work-tree operations inside the 
git directory), here is a patch.  I don't like it, but if you take it, 
I'll learn to live with it.

Ciao,
Dscho

-- snipsnap --
Reinstate the old behaviour when GIT_DIR is set and GIT_WORK_TREE is unset

The old behaviour was to unilaterally default to the cwd is the work tree 
when GIT_DIR was set, but GIT_WORK_TREE wasn't, no matter if we are inside 
the GIT_DIR, or if GIT_DIR is actually something like ../../../.git.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Feel free to change the commit message.

 setup.c |   50 +++++++++-----------------------------------------
 1 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/setup.c b/setup.c
index 4945eb3..7bcf4eb 100644
--- a/setup.c
+++ b/setup.c
@@ -189,53 +189,21 @@ int is_inside_work_tree(void)
 }
 
 /*
- * If no worktree was given, and we are outside of a default work tree,
- * now is the time to set it.
- *
- * In other words, if the user calls git with something like
- *
- *	git --git-dir=/some/where/else/.git bla
- *
- * default to /some/where/else as working directory; if the specified
- * git-dir does not end in "/.git", the cwd is used as working directory.
+ * set_work_tree() is only ever called if you set GIT_DIR explicitely.
+ * The old behaviour (which we retain here) is to set the work tree root
+ * to the cwd, unless overridden by the config, the command line, or
+ * GIT_WORK_TREE.
  */
 const char *set_work_tree(const char *dir)
 {
-	char dir_buffer[PATH_MAX], *rel = NULL;
-	static char buffer[PATH_MAX + 1];
-	int len, suffix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
-
-	/* strip the variable 'dir' of the postfix "/.git" if it has it */
-	len = strlen(dir);
-	if (len > suffix_len &&
-	    !strcmp(dir + len - suffix_len, "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
-		if ((len - suffix_len) >= sizeof(dir_buffer))
-			die("directory name too long");
-		memcpy(dir_buffer, dir, len - suffix_len);
-		dir_buffer[len - suffix_len] = '\0';
-
-		/* are we inside the default work tree? */
-		rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
-	}
+	char buffer[PATH_MAX + 1];
 
-	/* if rel is set, the cwd is _not_ the current working tree */
-	if (rel && *rel) {
-		if (!is_absolute_path(dir))
-			set_git_dir(make_absolute_path(dir));
-		dir = dir_buffer;
-		if (chdir(dir))
-			die("cannot chdir to %s: %s", dir, strerror(errno));
-		else
-			strcat(rel, "/");
-		inside_git_dir = 0;
-	} else {
-		rel = NULL;
-		dir = getcwd(buffer, sizeof(buffer));
-	}
-	git_work_tree_cfg = xstrdup(dir);
+	if (!getcwd(buffer, sizeof(buffer)))
+		die ("Could not get the current working directory");
+	git_work_tree_cfg = xstrdup(buffer);
 	inside_work_tree = 1;
 
-	return rel;
+	return NULL;
 }
 
 /*

Re: [PATCH] Fix install-doc-quick target

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I do not like the behaviour "be stupid and assume cwd to be the working 
> tree root, if GIT_DIR is set and GIT_WORK_TREE is not".
>
> It bears _all_ kind of stupid connotations.  Just imagine what would 
> happen with "git --git-dir=. add .".
>
> IMHO the new behaviour is _better_, since you can not shoot yourself in 
> the foot so easily.  Being able to safeguard against doing a work tree 
> operation inside the git directory is a direct and elegant consequence of 
> defaulting to $GIT_DIR/.. in case $GIT_DIR ends in "/.git", and no work 
> tree if $GIT_DIR does _not_ end in "/.git".
>
> The semantics "if GIT_DIR is set, just assume the cwd to be the work tree 
> root unilaterally" is _broken_ as far as I am concerned.

I am not disputing that.  I was just pointing out that this is a
change in semantics and we need to advertise it as such, and
more importantly, advise people how to adjust to the new (and
improved) world order.

Re: [PATCH] Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Sun, 5 Aug 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I do not like the behaviour "be stupid and assume cwd to be the working 
> > tree root, if GIT_DIR is set and GIT_WORK_TREE is not".
> >
> > It bears _all_ kind of stupid connotations.  Just imagine what would 
> > happen with "git --git-dir=. add .".
> >
> > IMHO the new behaviour is _better_, since you can not shoot yourself in 
> > the foot so easily.  Being able to safeguard against doing a work tree 
> > operation inside the git directory is a direct and elegant consequence of 
> > defaulting to $GIT_DIR/.. in case $GIT_DIR ends in "/.git", and no work 
> > tree if $GIT_DIR does _not_ end in "/.git".
> >
> > The semantics "if GIT_DIR is set, just assume the cwd to be the work tree 
> > root unilaterally" is _broken_ as far as I am concerned.
> 
> I am not disputing that.  I was just pointing out that this is a
> change in semantics and we need to advertise it as such, and
> more importantly, advise people how to adjust to the new (and
> improved) world order.

Thanks.  I will prepare a patch to ReleaseNotes, but that'll have to wait 
until tomorrow.  I'm pretty exhausted after the last night of msysGit 
hacking.

Ciao,
Dscho

Re: [PATCH] Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Sun, 5 Aug 2007, Junio C Hamano wrote:

>    If you say:
> 
> 	git --work-tree=. foo
> 
>    without saying anything about GIT_DIR, shouldn't we run the
>    usual .git/ discovery, going up the directories? 

Well, it seems natural.

The problem is that if you are in a bare repository, after 
setup_git_directory_gently() you will no longer be able to tell where you 
started from, but you are in the same directory that HEAD was found.

I had the following patch, but it breaks all kinds of tests :-(

Will keep you posted,
Dscho

-- snipsnap --

 setup.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/setup.c b/setup.c
index d87e4e1..c627623 100644
--- a/setup.c
+++ b/setup.c
@@ -291,7 +291,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env)
 				inside_work_tree = 0;
 			setenv(GIT_DIR_ENVIRONMENT, ".", 1);
-			return NULL;
+			goto ret;
 		}
 		chdir("..");
 		do {
@@ -311,6 +311,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!work_tree_env)
 		inside_work_tree = 1;
 	git_work_tree_cfg = xstrndup(cwd, offset);
+ret:
 	if (offset == len)
 		return NULL;
 
@@ -376,14 +377,15 @@ const char *setup_git_directory(void)
 	/* If the work tree is not the default one, recompute prefix */
 	if (inside_work_tree < 0) {
 		static char buffer[PATH_MAX + 1];
+		const char *git_dir = get_git_dir();
 		char *rel;
 
 		/*
 		 * When the git dir was determined automatically, it is
 		 * a relative path.
 		 */
-		if (!getenv(GIT_DIR_ENVIRONMENT))
-			set_git_dir(make_absolute_path(get_git_dir()));
+		if (!is_absolute_path(git_dir))
+			set_git_dir(make_absolute_path(git_dir));
 
 		if (retval && chdir(retval))
 			die ("Could not jump back into original cwd");

[PATCH] (Really) Fix install-doc-quick target

[Mark Levedahl]

Commit 00d8c51 caused git-ls-tree to be invoked from a directory where
GIT_DIR cannot be found and without that variable being set.

Signed-off-by: Mark Levedahl <mdl123@verizon.net>
---
 Documentation/install-doc-quick.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/install-doc-quick.sh b/Documentation/install-doc-quick.sh
index 07d227f..a0164fb 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -25,7 +25,7 @@ git checkout-index -a -f --prefix="$mandir"/
 
 if test -n "$GZ"; then
 	cd "$mandir"
-	for i in `git ls-tree -r --name-only $head`
+	for i in `git --git-dir="$GIT_DIR" ls-tree -r --name-only $head`
 	do
 		gzip < $i > $i.gz && rm $i
 	done
-- 
1.5.3.rc4.5.g4f0b5

Re: [PATCH] (Really) Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Mon, 6 Aug 2007, Mark Levedahl wrote:

> -	for i in `git ls-tree -r --name-only $head`
> +	for i in `git --git-dir="$GIT_DIR" ls-tree -r --name-only $head`

This awfully looks like hiding a bug.  It should _never_ be necessary to 
say '--git-dir="$GIT_DIR"'.

Ciao,
Dscho

Re: [PATCH] (Really) Fix install-doc-quick target

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Mon, 6 Aug 2007, Mark Levedahl wrote:
>
>> -	for i in `git ls-tree -r --name-only $head`
>> +	for i in `git --git-dir="$GIT_DIR" ls-tree -r --name-only $head`
>
> This awfully looks like hiding a bug.  It should _never_ be necessary to 
> say '--git-dir="$GIT_DIR"'.

Yes.  And 00d8c51 obviously "works for me", so there is
something different between what Mark and I are doing.  I cannot
tell what it is.

Re: [PATCH] (Really) Fix install-doc-quick target

[Mark Levedahl]

Junio C Hamano wrote:
> Yes.  And 00d8c51 obviously "works for me", so there is
> something different between what Mark and I are doing.  I cannot
> tell what it is.
>   
GZ=1 make quick-install-doc

...fails because git-ls-tree is called when cwd=$mandir which is nowhere 
under or related to $GIT_DIR.

Mark

Re: [PATCH] (Really) Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Mon, 6 Aug 2007, Mark Levedahl wrote:

> Junio C Hamano wrote:
> > Yes.  And 00d8c51 obviously "works for me", so there is
> > something different between what Mark and I are doing.  I cannot
> > tell what it is.
> >   
> GZ=1 make quick-install-doc
> 
> ...fails because git-ls-tree is called when cwd=$mandir which is nowhere under
> or related to $GIT_DIR.

Ah, I suspect you do not have the latest 'master' installed?

Ciao,
Dscho

Re: [PATCH] (Really) Fix install-doc-quick target

[Mark Levedahl]

Johannes Schindelin wrote:
> Ah, I suspect you do not have the latest 'master' installed?
>
> Ciao,
> Dscho
>
>   
git>git fetch origin
git>git show-ref origin/master
a76c2acb28146f5630592f2ba738c0ebf0f3c1c4 refs/remotes/origin/master
git>git checkout -b test origin/master
Branch test set up to track remote branch refs/remotes/origin/master.
Switched to a new branch "test"
git>GZ=1 make prefix=/usr quick-install-doc
GIT_VERSION = 1.5.3.rc4.16.ga76c2
make -C Documentation quick-install
make[1]: Entering directory `/usr/src/git/Documentation'
rm -f doc.dep+ doc.dep
perl ./build-docdep.perl >doc.dep+
mv doc.dep+ doc.dep
make -C ../ GIT-VERSION-FILE
make[2]: Entering directory `/usr/src/git'
make[2]: `GIT-VERSION-FILE' is up to date.
make[2]: Leaving directory `/usr/src/git'
make[1]: Leaving directory `/usr/src/git/Documentation'
make[1]: Entering directory `/usr/src/git/Documentation'
make -C ../ GIT-VERSION-FILE
make[2]: Entering directory `/usr/src/git'
make[2]: `GIT-VERSION-FILE' is up to date.
make[2]: Leaving directory `/usr/src/git'
sh ./install-doc-quick.sh origin/man /usr/share/man
fatal: Not a git repository
make[1]: Leaving directory `/usr/src/git/Documentation'
git>

Re: [PATCH] (Really) Fix install-doc-quick target

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Junio C Hamano wrote:
>> Yes.  And 00d8c51 obviously "works for me", so there is
>> something different between what Mark and I are doing.  I cannot
>> tell what it is.
>>
> GZ=1 make quick-install-doc
>
> ...fails because git-ls-tree is called when cwd=$mandir which is
> nowhere under or related to $GIT_DIR.

Oops.  I am blind.  That's right.  That command does a chdir on
its own.

How does this sound?

---
diff --git a/Documentation/install-doc-quick.sh b/Documentation/install-doc-quick.sh
index 07d227f..bc170f0 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -24,10 +24,10 @@ git read-tree $head
 git checkout-index -a -f --prefix="$mandir"/
 
 if test -n "$GZ"; then
-	cd "$mandir"
-	for i in `git ls-tree -r --name-only $head`
+	git ls-tree -r --name-only $head |
+	while read path
 	do
-		gzip < $i > $i.gz && rm $i
+		gzip "$mandir/$path"
 	done
 fi
 rm -f "$GIT_INDEX_FILE"

Re: [PATCH] (Really) Fix install-doc-quick target

[Mark Levedahl]

Subject:   Re: [PATCH] (Really) Fix install-doc-quick target
To:        Junio C Hamano <gitster@pobox.com>
Cc:        Johannes Schindelin <Johannes.Schindelin@gmx.de>,Mark 
Levedahl <mdl123@verizon.net>,Git Mailing List <git@vger.kernel.org>,Ren 
Scharfe <rene.scharfe@lsrfire.ath.cx>
Bcc:
Reply-To:
Newsgroup:
-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-
Junio C Hamano wrote:
 > Mark Levedahl <mlevedahl@gmail.com> writes:
 >
 >
 > How does this sound?
 >
 > ---
 > diff --git a/Documentation/install-doc-quick.sh 
b/Documentation/install-doc-quick.sh
 > index 07d227f..bc170f0 100755
 > --- a/Documentation/install-doc-quick.sh
 > +++ b/Documentation/install-doc-quick.sh
 > @@ -24,10 +24,10 @@ git read-tree $head
 >  git checkout-index -a -f --prefix="$mandir"/
 >
 >  if test -n "$GZ"; then
 > -    cd "$mandir"
 > -    for i in `git ls-tree -r --name-only $head`
 > +    git ls-tree -r --name-only $head |
 > +    while read path
 >      do
 > -        gzip < $i > $i.gz && rm $i
 > +        gzip "$mandir/$path"
 >      done
 >  fi
 >  rm -f "$GIT_INDEX_FILE"
 >
 >

Maybe this instead, many fewer gzip invocations, happily overwrites the 
old man pages already installed...

---
diff --git a/Documentation/install-doc-quick.sh 
b/Documentation/install-doc-quick.sh
index 07d227f..45f78fa 100755
--- a/Documentation/install-doc-quick.sh
+++ b/Documentation/install-doc-quick.sh
@@ -24,10 +24,6 @@ git read-tree $head
 git checkout-index -a -f --prefix="$mandir"/

 if test -n "$GZ"; then
-    cd "$mandir"
-    for i in `git ls-tree -r --name-only $head`
-    do
-        gzip < $i > $i.gz && rm $i
-    done
+    printf "$mandir/%s\n" $(git ls-tree -r --name-only $head) | xargs 
gzip -f
 fi
 rm -f "$GIT_INDEX_FILE"

Re: [PATCH] (Really) Fix install-doc-quick target

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> +    printf "$mandir/%s\n" $(git ls-tree -r --name-only $head) | xargs
> gzip -f

No risk that ls-tree output is too long to fit within the exec
args limit to run printf?

Re: [PATCH] (Really) Fix install-doc-quick target

[René Scharfe]

Junio C Hamano schrieb:
> Mark Levedahl <mlevedahl@gmail.com> writes:
> 
>> +    printf "$mandir/%s\n" $(git ls-tree -r --name-only $head) | xargs
>> gzip -f
> 
> No risk that ls-tree output is too long to fit within the exec
> args limit to run printf?

Perhaps this instead?

   git ls-tree -r --name-only $head | (cd "$mandir" && xargs gzip -f)

René

Re: [PATCH] (Really) Fix install-doc-quick target

[Johannes Schindelin]

Hi,

On Tue, 7 Aug 2007, Ren? Scharfe wrote:

> Junio C Hamano schrieb:
> > Mark Levedahl <mlevedahl@gmail.com> writes:
> > 
> >> +    printf "$mandir/%s\n" $(git ls-tree -r --name-only $head) | xargs
> >> gzip -f
> > 
> > No risk that ls-tree output is too long to fit within the exec
> > args limit to run printf?
> 
> Perhaps this instead?
> 
>    git ls-tree -r --name-only $head | (cd "$mandir" && xargs gzip -f)

I would have done

	git ls-tree -r --name-only $head | sed "s|^|$mandir|" | xargs gzip -f

but I like your version better.

Thanks for teaching me,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

Mark Levedahl wrote:
> Johannes Schindelin wrote:
>> Could it be that you did not find this commit by bisecting the 
>> issue?  I highly doubt that said commit changes anything in the build 
>> process.
>>
> git>git bisect good
> 6490a3383f1d0d96c122069e510ef1af1d019fbb is first bad commit
> commit 6490a3383f1d0d96c122069e510ef1af1d019fbb
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Thu Aug 2 15:10:56 2007 -0700
>
>    Fix work-tree related breakages
Sorry - I hit send too fast on the previous mail. The relevant build 
lines are:

GIT_INDEX_FILE=`pwd`/.quick-doc.index
export GIT_INDEX_FILE
rm -f "$GIT_INDEX_FILE"
git read-tree $head
git checkout-index -a -f --prefix="$mandir"/

After this commit, git checkout-index does not write the man files into 
$mandir, before this commit, it does.

Mark Levedahl

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Mark Levedahl wrote:

> git checkout-index -a -f --prefix="$mandir"/

Just to hazard a guess... you see an error message there, such as "cannot 
chdir to <blablabla>: No such file or directory"?

It's kinda hard to debug this with incomplete information.

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

Johannes Schindelin wrote:
> Hi,
>
> On Sat, 4 Aug 2007, Mark Levedahl wrote:
>
>   
>> git checkout-index -a -f --prefix="$mandir"/
>>     
>
> Just to hazard a guess... you see an error message there, such as "cannot 
> chdir to <blablabla>: No such file or directory"?
>
> It's kinda hard to debug this with incomplete information.
>
> Ciao,
> Dscho
>
>
>   
Nope - no error messages, nada. The trace output I sent you is 
absolutely uninformative as well. There is absolutely no visible 
difference between the version where this works (the previous commit) 
and the version where it does not, except that in the latter case the 
files are not written out (or at least not to where they should be 
written nor to any other place I can find.

If you have any suggestions on how I can help debug this, I am more than 
willing but not at all familiar with the internal workings of git.

Mark

Re: rc4 - make quick-install-doc is broken

[René Scharfe]

Mark Levedahl schrieb:
> git>git bisect good
> 6490a3383f1d0d96c122069e510ef1af1d019fbb is first bad commit

I've started a bisect run myself and ended up at a different commit,
viz. e90fdc39b6903502192b2dd11e5503cea721a1ad ("Clean up work-tree
handling").  Hmm.  I guess this candidate has a greater chance of
actually being the culprit than yours. ;-)

I can't offer a fix, but I think I've captured install-doc-quick.sh's
problem in a test script (see below).  It fails with e90fdc3 (and
master) but succeeds with e90fdc3^.

Apparently checkout-index (and ls-files, but this is not used by the
install script) can now be confused by running it from inside an
untracked directory.

Johannes, does this help you in finding out what's going on here?

René


diff --git a/dev/null b/t/t1502-untracked.sh
new file mode 100755
index 0000000..3fbdb02
--- /dev/null
+++ b/t/t1502-untracked.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='accessing the index from an untracked directory'
+. ./test-lib.sh
+
+pwd=$(pwd)
+GIT_DIR=$pwd/.git
+export GIT_DIR
+
+test_expect_success 'setup' '
+	git init &&
+	echo some file content >some_file &&
+	git add some_file &&
+	git commit -m "added a file" some_file &&
+	mkdir untracked
+'
+
+test_expect_success 'test read-tree from an untracked directory' '
+	(                GIT_INDEX_FILE=$pwd/idx-a git read-tree HEAD) &&
+	(cd untracked && GIT_INDEX_FILE=$pwd/idx-b git read-tree HEAD) &&
+	git diff --binary idx-a idx-b
+'
+
+test_expect_success 'test checkout-index from an untracked directory' '
+	(                git checkout-index -a --prefix=$pwd/a/) &&
+	(cd untracked && git checkout-index -a --prefix=$pwd/b/) &&
+	git diff a/ b/
+'
+
+test_expect_success 'test ls-files from an untracked directory' '
+	(                git ls-files) >ls-files-a &&
+	(cd untracked && git ls-files) >ls-files-b &&
+	git diff ls-files-a ls-files-b
+'
+
+test_done

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Ren? Scharfe wrote:

> Mark Levedahl schrieb:
> > git>git bisect good
> > 6490a3383f1d0d96c122069e510ef1af1d019fbb is first bad commit
> 
> I've started a bisect run myself and ended up at a different commit,
> viz. e90fdc39b6903502192b2dd11e5503cea721a1ad ("Clean up work-tree
> handling").  Hmm.  I guess this candidate has a greater chance of
> actually being the culprit than yours. ;-)
> 
> I can't offer a fix, but I think I've captured install-doc-quick.sh's
> problem in a test script (see below).  It fails with e90fdc3 (and
> master) but succeeds with e90fdc3^.
> 
> Apparently checkout-index (and ls-files, but this is not used by the
> install script) can now be confused by running it from inside an
> untracked directory.
> 
> Johannes, does this help you in finding out what's going on here?

Thanks for doing this.  I will work on this in a few minutes; at the 
moment I am occupied with msysGit...

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[Mark Levedahl]

René Scharfe wrote:
> Mark Levedahl schrieb:
>   
>> git>git bisect good
>> 6490a3383f1d0d96c122069e510ef1af1d019fbb is first bad commit
>>     
>
> I've started a bisect run myself and ended up at a different commit,
> viz. e90fdc39b6903502192b2dd11e5503cea721a1ad ("Clean up work-tree
> handling").  Hmm.  I guess this candidate has a greater chance of
> actually being the culprit than yours. ;-)
>   

Nak - at commit e90fdc... it works fine for me here. However, one 
problem is the following lines in setup.c/set_work_tree() that appear 
until 6490a3

            strncpy(dir_buffer, dir, len - postfix_len);

        /* are we inside the default work tree? */
        rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);

Note that dir_buffer is not null terminated in the above code, that bug 
has been around for a while, and you certainly could run into a 
different problem than I did as the results are essentially undefined 
here because of that bug.. The key innovation for the current discussion 
in commit 6490a33 is that dir_buffer is null terminated after the copy, 
causing a different but repeatable result from get_relative_cwd.

I think this ultimately exposes a bug in 
builtin-checkout-index/checkout_all wherein the latter does not 
explicitly honor the given --prefix option that is passed along in 
state.  Specifically, on each pass the following test passes

        if (prefix && *prefix &&
            (ce_namelen(ce) <= prefix_length ||
             memcmp(prefix, ce->name, prefix_length)))
            continue;

so the code never attempts to write the file out. I cannot fathom what 
this test is trying to discern and have to leave it to others more 
familiar with the design to figure out the right course. For this 
particular use, the passed in prefix is irrelevant, the correct path to 
write to is in state.

Mark

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sat, 4 Aug 2007, Ren? Scharfe wrote:

> I can't offer a fix, but I think I've captured install-doc-quick.sh's
> problem in a test script (see below).  It fails with e90fdc3 (and
> master) but succeeds with e90fdc3^.

It succeeds here... (without the patch I sent out, of course.)

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[René Scharfe]

Johannes Schindelin schrieb:
> Hi,
> 
> On Sat, 4 Aug 2007, Ren? Scharfe wrote:
> 
>> I can't offer a fix, but I think I've captured install-doc-quick.sh's
>> problem in a test script (see below).  It fails with e90fdc3 (and
>> master) but succeeds with e90fdc3^.
> 
> It succeeds here... (without the patch I sent out, of course.)

I assume you tested e90fdc3 and e90fdc3^; what about 4f8f03d (current HEAD)?

Thanks,
René

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

On Sun, 5 Aug 2007, Ren? Scharfe wrote:

> Johannes Schindelin schrieb:
>
> > On Sat, 4 Aug 2007, Ren? Scharfe wrote:
> > 
> >> I can't offer a fix, but I think I've captured install-doc-quick.sh's 
> >> problem in a test script (see below).  It fails with e90fdc3 (and 
> >> master) but succeeds with e90fdc3^.
> > 
> > It succeeds here... (without the patch I sent out, of course.)
> 
> I assume you tested e90fdc3 and e90fdc3^; what about 4f8f03d (current 
> HEAD)?

I thought I tested on master + --new-workdir... But I did not.

And indeed, it fails.

Sorry,
Dscho

Re: rc4 - make quick-install-doc is broken

[Johannes Schindelin]

Hi,

your test script is slightly wrong...

First you "git init", with GIT_DIR in $(pwd)/.git, i.e. the default.  But 
then, you need not do this, test scripts are called when git init was 
already called.

Then you make an untracked directory called untracked/.  Tradition 
dictates that when we're in that directory, we get the prefix 
"untracked/", because we might add a file, or reference a file in another 
branch, where that directory is _not_ untracked.

So it is expected that checkout-index and ls-files behave differently 
in a subdirectory (even if that is currently untracked).

It seems a bit counterintuitive that read-tree succeeds, but really, 
read-tree is only a commit -> index operation, which should not care about 
the current prefix.  So it is fine.

Checkout-index, instead, is an index -> working tree operation, and for 
most of these, we care about the current prefix (so that you can say git 
checkout-index file1, where file1 is in the current directory, which is 
_not_ the working tree root).

Ciao,
Dscho

Re: rc4 - make quick-install-doc is broken

[René Scharfe]

Johannes Schindelin schrieb:
> Then you make an untracked directory called untracked/.  Tradition 
> dictates that when we're in that directory, we get the prefix 
> "untracked/", because we might add a file, or reference a file in another 
> branch, where that directory is _not_ untracked.

The test is modelled after the install script; Documentation/ (the CWD
during 'make quick-install-doc') is not tracked in branch origin/man.

> So it is expected that checkout-index and ls-files behave differently 
> in a subdirectory (even if that is currently untracked).
> 
> It seems a bit counterintuitive that read-tree succeeds, but really, 
> read-tree is only a commit -> index operation, which should not care about 
> the current prefix.  So it is fine.
> 
> Checkout-index, instead, is an index -> working tree operation, and for 
> most of these, we care about the current prefix (so that you can say git 
> checkout-index file1, where file1 is in the current directory, which is 
> _not_ the working tree root).

OK, makes sense.

Thanks,
René