juergbi commented on issue #2178:
URL: https://github.com/apache/buildstream/issues/2178#issuecomment-5730594319

   While this issue is about not loading anything from outside the element 
directory, symlinks can trigger other issues. A symlinked element currently 
(with implicit symlink following) behaves as if the element was copied. If the 
symlink and the target are part of the same pipeline, they will have different 
cache keys, resulting in duplicated builds. I can't think of a case where this 
is actually wanted, can anyone?
   
   We already support `type: link` as symbolic link that that avoids this issue 
and also works across junctions. While we theoretically could support symlinks 
behaving like `type: link`, I'm not convinced we should. I suspect we wouldn't 
catch all edge cases in the initial implementation (we had bugs with `type: 
link` in the past), and with `type: link` already being available, I don't 
think there is a real need to make things more complicated. That said, 
disallowing symlinks may not be completely trivial either (without some helper 
like `Directory`). If it turns out that safely supporting symlinks to behave 
like `type: link` is not more complex than disallowing symlinks, I might 
reconsider.
   
   `..` and `.` don't seem very useful with element paths being relative to the 
element directory, but with normalization (already added in #2133) and 
directory escape prevention, we don't necessarily have to reject them.
   
   Using `Directory` could be interesting, besides the built-in anti-escape 
mechanism, it potentially might allow to directly use `CasBasedDirectory` for 
subprojects instead of having to stage subprojects in the filesystem (but that 
would be a larger change, affecting more areas than element loading, if 
possible at all).
   
   One concern is also compatibility. There might be projects that rely on 
symlinks working, despite the shortcoming mentioned in the first paragraph. 
While fixed symlink support would technically still come with a behavior 
difference, it would hopefully just be an improvement, not a compatibility 
issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to