Opened 9 years ago

Closed 9 years ago

#477 closed defect (fixed)

project name is incorrect when root is a symbolic link

Reported by: Peter Johansson Owned by: Peter Johansson
Priority: trivial Milestone: svndigest 0.8.1
Component: output Version: 0.8
Keywords: Cc:

Description

In the trunk test run the --root is a symbolic link

svndigest -> ../trunk

In this case we expect the project name, the very root of path (in upper left corner of output) to be svndigest, but it becomes trunk as we can see in the output here

http://cbbp.thep.lu.se/~jari/svndigest/svndigest-trunk/blame/all/total/index.html

Change History (11)

comment:1 Changed 9 years ago by Peter Johansson

Owner: changed from Jari Häkkinen to Peter Johansson
Status: newassigned

comment:2 Changed 9 years ago by Peter Johansson

Milestone: svndigest 0.9svndigest 0.8.1
Version: trunk0.8

The bug confirmed with 0.8.1pre

comment:3 Changed 9 years ago by Peter Johansson

The problem is that we use getcwd to get the absolute path to --root argument. For some reason in the example in description,

chdir(trunk)
getcwd()

will return /full/path/to/svndigest not /full/path/to/trunk.

comment:4 Changed 9 years ago by Peter Johansson

Hm, this is as specified in posix. The returned string from getcwd should contain no symbolic link.

http://www.opengroup.org/onlinepubs/009695399/functions/getcwd.html

comment:5 Changed 9 years ago by Peter Johansson

A related bug:

$ cd /home/peter/projects/svndigest
$ ln -s . symlink
$ ./bin/svndigest --root symlink/..
svndigest: '/home/peter/projects' is not a working copy

while I expected this to be equivalent with

$ ./bin/svndigest --root .

Am I missing something?

comment:6 in reply to:  5 ; Changed 9 years ago by Jari Häkkinen

Replying to peter:

Am I missing something?

Yes. symlink points to the current directory therefore symlink/.. is equal to ./.. the parent directory.

comment:7 in reply to:  6 Changed 9 years ago by Peter Johansson

Replying to jari:

Replying to peter:

Am I missing something?

Yes. symlink points to the current directory therefore symlink/.. is equal to ./.. the parent directory.

Hm, OK. I tried with cd symlink/.. before being the same as cd symlink; cd ... But now I tried ls symlink/.. and cp -rv symlink/.. ~/tmp/. and both behave as you argue is natural. I rest my case.

Interestingly

$ svn info symlink/..
Path: .
URL: http://dev.thep.lu.se/yat/svn/trunk
[cut]

comment:8 Changed 9 years ago by Peter Johansson

Currently, Parameter::root() returns the dereferenced (no links) absolute path of --root argument. I modified this so when --root argument we are not dereferencing the link but only make the path absolute. This would mean that we got the wanted filename also when --root argument is a symlink.

The problem is that the root Directory (tree in main) will return ignore()=true because it is now a symlink, which implies entire tree is ignored. Not what we wanted.

Do we want to allow root tree to be symlink or should we barf out in that case? If we do wanna allow it, I need to go back to the drawing board. If we don't wanna allow it, I think we should do the change in trunk instead and not in v0.8.1.

comment:9 in reply to:  8 Changed 9 years ago by Jari Häkkinen

Replying to peter:

Do we want to allow root tree to be symlink or should we barf out in that case? If we do wanna allow it, I need to go back to the drawing board. If we don't wanna allow it, I think we should do the change in trunk instead and not in v0.8.1.

Hm, my first reaction is that we should allow symlinks for the root. Because symlinks are frequently used in Unix and it may be annoying for the user to have to adapt to svndigest behaviour. But then again, there are many workarounds possible outside of svndigest so it is not the most important problem to solve. Do you have an estimate on how much work it would be to allow root to be a symlink?

comment:10 Changed 9 years ago by Peter Johansson

I have a next plan; if that works out it shouldn't be too much work.

The thing is that we wanna use the symlink sometimes and the symlinkee sometimes. If I've understood correctly we wanna use the symlink when calculating the target directory and we wanna use it in when output the path in top left corner (as described in initial description). However, when we create the tree, we wanna use the symlinkee (i.e. the dereferenced path).

My plan is to have two functions in Parameter. One that returns the dereferenced absolute path as now. And one that returns the same if not a symlink and the full path to the symlink if a symlink. The first function should be used as path when creating tree and the latter function is used when calculating the target directory and static variable Node::project_. My hope is that this static variable is only used for the outputting the anchor path and then we should be home. If not, we are probably in trouble (and should perhaps consider not allowing...).

Let me fly a round to check what the terrain looks like behind the skyline.

comment:11 Changed 9 years ago by Peter Johansson

Resolution: fixed
Status: assignedclosed

(In [1246]) fixes #477

Note: See TracTickets for help on using tickets.