indexing files through symbolic links

2 views
Skip to first unread message

Elliott Hughes

unread,
Mar 21, 2009, 9:34:38 PM3/21/09
to evergre...@googlegroups.com
what's the motivation for this method in WorkspaceFileList?

private boolean isSymbolicLinkWithinWorkspace(File file) {
if (FileUtilities.isSymbolicLink(file) == false) {
return false;
}
try {
String canonicalFileName =
file.getCanonicalFile().toString();
String canonicalWorkspaceRootDirectory =
workspace.getCanonicalRootDirectory();
return
canonicalFileName.startsWith(canonicalWorkspaceRootDirectory);
} catch (IOException ex) {
// (If we can't find the root directory of the workspace,
then what are we scanning?)
// If we can't find the target of a symbolic link, then we
can't very well edit it.
return true;
}
}

why doesn't the [only] call to it just say FileUtilities.isSymbolicLink
instead?

private void scanDirectory(File directory, FileIgnorer
fileIgnorer, ArrayList<String> result) {
File[] files = directory.listFiles();
if (files == null) {
return;
}
for (File file : files) {
boolean isDirectory = file.isDirectory();
if (fileIgnorer.isIgnored(file, isDirectory)) {
continue;
}
if (isSymbolicLinkWithinWorkspace(file)) {
continue;
}
if (isDirectory) {
scanDirectory(file, fileIgnorer, result);
} else {
result.add(file.toString().substring(prefixCharsToSkip));
}
}
}

this code is causing me trouble because if you have a link to a link to
somewhere outside the workspace root, we'll accidentally trundle off and
index that. i could make it recursive, so as long as we're looking at a
link, we follow it and do the test again. but i can't think why we don't
just want FileUtilities.isSymbolicLink instead.

from the history, it looks like this was *added* for the very reason i
want to remove it: links to salma-hayek from within a repository. this
seems like a mistake. if you want to index (say) terminator and
salma-hayek together, shouldn't you put them in a workspace together like
mad does?

--
Elliott Hughes, http://www.jessies.org/~enh/


Martin Dorey

unread,
Mar 31, 2009, 5:08:18 PM3/31/09
to evergre...@googlegroups.com

> if you have a link to a link to

> somewhere outside the workspace root, we'll accidentally trundle off and

> index that

 

That description of the problem scenario misled me on two counts.  Firstly, you only need to have a link to somewhere outside the workspace root for us to trundle off - you don't need a link to a link.  Secondly, while it might be unfortunate, it's not an accidental trundling off - that was the very purpose of that code, as later stated:

 

> from the history, it looks like this was *added* for the very reason i

> want to remove it: links to salma-hayek from within a repository.

 

From the history we can also see that the code was written for Ed.  Despite being the author, I've no use for it and no objection to it being removed.

 

2007-01-31 20:25:16 -0800 mad (1060)

 

src/e/edit/WorkspaceFileList.java: Fix Ed's problem caused by my change here the other day without breaking what I fixed.  He has a symlink to a directory like ../salma-hayek from within a workspace root like Evergreen.  Now we follow such symlinks without following symlinks within the workspace, which would give us duplicate paths to the same file and (potentially) cycles.

Elliott Hughes

unread,
Apr 1, 2009, 1:04:49 AM4/1/09
to evergreen-users


On Mar 31, 2:08 pm, "Martin Dorey" <mdo...@bluearc.com> wrote:
> > if you have a link to a link to
> > somewhere outside the workspace root, we'll accidentally trundle off
> and
> > index that
>
> That description of the problem scenario misled me on two counts.
> Firstly, you only need to have a link to somewhere outside the workspace
> root for us to trundle off - you don't need a link to a link.  Secondly,
> while it might be unfortunate, it's not an accidental trundling off -
> that was the very purpose of that code, as later stated:

yeah, i tend to write these things as i investigate them.

> > from the history, it looks like this was *added* for the very reason i
> > want to remove it: links to salma-hayek from within a repository.
>
> From the history we can also see that the code was written for Ed.
> Despite being the author, I've no use for it and no objection to it
> being removed.

i was hoping Ed would remind us what his problem was, and whether he
still has it.

i'd like to not follow symbolic links to have a way to reference other
projects _without_ dragging them in, which i don't think you can do
otherwise. the opposite seems easy; use a hard link or check out
multiple projects into a directory and make that directory a workspace
root. (certainly the latter works as an excellent solution to the
presumably analogous problem in your check-in comment. we even
document it, though it's not in the manual or FAQ; strangely it
appears to have been left on the "features" list.)

plus "we don't follow symbolic links" (which is what i thought) is a
slightly simpler rule than "we don't follow symbolic links unless
following them (possibly transitively) to their (final) target takes
us above the workspace root.

if Ed's no longer making use of this or would be happy with the
recommended solution, i'll take it out. otherwise i'll add a property
or something.

--elliott

Martin Dorey

unread,
Apr 1, 2009, 12:38:03 PM4/1/09
to evergre...@googlegroups.com

> the opposite seems easy; use a hard link or check out

> multiple projects into a directory and make that directory a workspace

> root. (certainly the latter works

 

Good job, cos the former doesn't :).  Each directory is required to have a unique parent, perhaps so lookup .. works, so hard links to directories aren't allowed.

 

(BlueArc's releases each use a pair of large source code control work areas.  We wouldn't want to check out or build multiple copies of these but I can imagine wanting two indexes - one for each work area individually and one for the pair of related areas.  Various scripts insist that all these directories, for multiple releases, live under a single common parent.  That would kibosh the latter idea except that those scripts already have to cope with the "directories" being symlinks, so that we can use multiple disks.  Thus we could structure the directories in the more nested style - release/work-area - then symlink to "work-area" from somewhere else and index at both the "release" and the "work-area" level.  So the latter approach would work fine there too.)

 

> otherwise i'll add a property

 

Blech.  Perhaps the description of his use-case will suggest...

 

> or something.

 

... that we can all live with.

 

Paging Mr Porter.  Third call for Mr Porter.

Ed Porter

unread,
Apr 1, 2009, 2:21:42 PM4/1/09
to evergre...@googlegroups.com

On 1 Apr 2009, at 18:38, Martin Dorey wrote:
>
>
> Blech. Perhaps the description of his use-case will suggest...
>
> > or something.
>
> ... that we can all live with.
>
> Paging Mr Porter. Third call for Mr Porter.

Oh right.

I can't remember what it's for, but no doubt I'll will do as soon as
you take it out.

-ed

Elliott Hughes

unread,
Apr 1, 2009, 2:52:10 PM4/1/09
to evergreen-users
to save a round trip (and possibly save yourself some hassle), do you
want to try it first and let us know what, if anything, it breaks for
you? i was thinking of replacing the call to
isSymbolicLinkWithinWorkspace (line 189 of WorkspaceFileList.java)
with a call to FileUtilities.isSymbolicLink instead.

i can send a patch, or just commit. whatever you'd prefer.

--elliott

Martin Dorey

unread,
Apr 1, 2009, 2:55:19 PM4/1/09
to evergre...@googlegroups.com
I didn't read this until I'd committed. Oh well.

-----Original Message-----
From: evergre...@googlegroups.com
[mailto:evergre...@googlegroups.com] On Behalf Of Elliott Hughes
Sent: Wednesday, April 01, 2009 11:52
To: evergreen-users
Subject: Re: indexing files through symbolic links


Ed Porter

unread,
Apr 1, 2009, 3:01:00 PM4/1/09
to evergre...@googlegroups.com

On 1 Apr 2009, at 20:55, Martin Dorey wrote:

>
> I didn't read this until I'd committed. Oh well.

Doesn't matter. I'll let you know, and/or modify my workspaces, if it
causes trouble.

-ed

Reply all
Reply to author
Forward
0 new messages