Re: [PATCH 3/8] Clean up work-tree handling

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 3/8] Clean up work-tree handling

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:24

Johannes Schindelin [off-list ref] writes:
quoted
Well, one of the conditions to accept the worktree stuff was not
to break anybody who never ever uses worktree.  So if we can
keep the UP-ness of cdup, it would be much better.
One could record if the work tree was changed from the default one, and 
do the old thing in that case.
This really should not be something you have to bend over with
special code to support.  One thing I think people's scripts
expect is that cdup is what the name suggests, "how much do I go
*up* in order to go to the top of the working tree", and if the
answer is "nothing", they can tell that they are already at the
top of the tree.

Now, I think it is fair to say that if your worktree is
somewhere totally unrelated to your cwd, no amount of going up
will take you to the top.  IOW, you have to come down after
going up some levels.  In such a case, it is easier to code the
implementation of --show-cdup to give an absolute path.

But in that case you are not even in the working tree to begin
with, aren't you?  Does git need to support that?  What should
"prefix = setup_git_dir()" do and return, in order to come up
with the string to prepend to paths relative to the current
directory the users give us?  I would say if your cwd is not
inside your working tree, then --show-cdup and --show-prefix
should error out, saying "Not in the working tree".  These
options are about translating paths relative to your cwd to
relative to the top of the working tree, so if you are outside a
working tree, they do not make much sense.

Also I think the implementation of --show-prefix needs to work
in terms of relative relationship between cwd and the top
already, so I am not sure where your aversion of keeping the
current cdup behaviour is coming from.

I was hoping that --show-cdup is rarely used, but unfortunately
cd_to_toplevel() in git-sh-setup.sh is not even the only in-tree
user of --show-cdup; it checks if it is empty to avoid chdir()
unnecessarily.  There are others:

 - emacs/git.el uses it as part of expand-file-name, so I think
   giving an absolute path back is Ok.

 - contrib/fast-import/git-p4 gives it to os.chdir() after
   checking if the length is positive; this should be Ok
   although it would involve an unnecessary chdir() if the
   process is already at the top with this change.

 - contrib/p4import/git-p4import.py: the same story as git-p4.

 - git-svn.perl: the same story as above.

Re: [PATCH 3/8] Clean up work-tree handling

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:24

Hi,

On Fri, 27 Jul 2007, Junio C Hamano wrote:
Now, I think it is fair to say that if your worktree is somewhere 
totally unrelated to your cwd, no amount of going up will take you to 
the top.  IOW, you have to come down after going up some levels.  In 
such a case, it is easier to code the implementation of --show-cdup to 
give an absolute path.

But in that case you are not even in the working tree to begin
with, aren't you?  Does git need to support that?
I'd say yes.

It is utterly _inconvenient_ to have to cd to the working tree when you 
just want to check the status, for example.  And git already knows about 
the work-tree!

But you got me convinced about the relative path: it is true that nobody 
who has not set core.worktree should be affected.

So I will do something like

	if (!inside_work_tree()) {
		puts(get_git_work_tree());
		continue;
	}
	[do the old thing of outputting ../../[...]]

In fact, I had this in an unpublished version of the patch, and decided 
that I could remove more lines without breaking the test suite.

Heck, I'll even add a test case to make sure that behavior is maintained.  
Okay?

Ciao,
Dscho

P.S.: I'll be offline for a few hours, but then come back to finish it up.

Re: [PATCH 3/8] Clean up work-tree handling

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:24

Hi,

I still have a problem with what should happen if both "core.bare == true" 
and "core.worktree = /some/where/over/the/rainbow".  Should it be bare, or 
should it have a working tree?

So here is what I plan to support so far:

--work-tree= overrides GIT_WORK_TREE, which overrides core.worktree, which 
overrides core.bare, which overrides GIT_DIR/.. when GIT_DIR ends in 
/.git, which overrides the directory in which .git/ was found.

Does that look okay?

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help