[PATCH] D55606: [gn build] Add infrastructure to create symlinks and use it to create lld's symlinks

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: llvm/utils/gn/build/symlink_or_copy.py:3
+
+"""Symlinks, or on Windows copies, an existing file to a second location.
+
----------------
smeenai wrote:
> thakis wrote:
> > smeenai wrote:
> > > thakis wrote:
> > > > thakis wrote:
> > > > > smeenai wrote:
> > > > > > Starting with Windows 10 RS2 (version 1703, released April 2017), you can create symlinks without needing to elevate to administrator if you have developer mode configured. I would imagine that's a reasonably common configuration for LLVM devs, and the symlinks end up saving a bunch of space. Do you think it's worth considering (not necessarily as a part of this patch; it could be done later) attempting to create a symlink even on Windows, and falling back to a copy if you get a permission denied error? I've been meaning to implement something similar on the CMake side.
> > > > > I think that'd be great. We probably wouldn't want this for releases (since users might have older Windows versions), but since the gn build is primarily for day-to-day development it makes a lot of sense to me.
> > > > > 
> > > > > I'd try that in a follow-up so I can test it on my Windows box if that's alright. (Or you could try making the change, it should be a < 10 line change in just this script.)
> > > > (ps: Once we no longer need to worry about old Windows versions that don't have symlinks, we can use the LHS on https://github.com/nico/llvm-project-20170507/commit/f24e0f0152d22eecad27f63b58149942c4b9bc22 instead)
> > > I'll play around with this ... I've been meaning to experiment with the gn build for a while :)
> > > 
> > > I actually think it would be fine to use symlinks for releases as well. Symlinks have been around since Windows Vista, but you had to elevate to administrator to be able to create or modify them, so they were rarely used. Once created, you could use them without elevation though. Windows 10 RS2 removed the elevation requirement when you were in developer mode, making them much more convenient. See https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ for more details (which also reminds me, we'd have to make sure Python was actually passing `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE` to its `CreateSymbolicLink` calls for this to work...)
> > Ah, I didn't know about using symlinks not requiring privileges! That would be super nice for distribution then, actually.
> > 
> > I'd expect this would have to go through ctypes.windll.kernel32.CreateSymbolicLinkW instead of os.symlink; py2.7 probably precedes SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE.
> Yup, Python 2.7 doesn't expose `os.symlink` at all on Windows, and even 3.7.1 doesn't use SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE (which is disappointing).
https://github.com/python/cpython/pull/3652 will fix `os.symlink`, but directly calling ctypes.windll.kernel32.CreateSymbolicLinkW in the meantime will work. I'll put up a patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55606/new/

https://reviews.llvm.org/D55606