filetools.copy_dir directly calls shutil.copytree with symlinks=False by default which means that symlinks will be followed and their contents copied instead of the symlinks
Also many EasyConfigs use the parameter keepsymlinks which is also set to False by default and often forgotten. E.g. for the EasyBlocks MakeCp and Tarball
I propose to switch the default to copy symlinks as symlinks instead and not resolve them. This has the following benefits:
Especially for Tarball I'd say the expectation is that the result is the same as when directly extracting the tarball to the destination
It works more often. The current default may needlessly copy stuff or even result in full failures. Happened during installation of Spack which has a recursive symlink (due to some localization stuff)
It works for absolute and broken (at install time) symlinks. I doubt we'd ever want to copy the contents of absolute symlinks, do we? And for broken ones they may be resolved later, e.g. to user home directories or similar
@Flamefire I agree that we should switch to retaining symlinks rather than resolving them, but that's a change we can/should only make in EasyBuild 5.0 imho.
filetools.copy_dir
directly callsshutil.copytree
withsymlinks=False
by default which means that symlinks will be followed and their contents copied instead of the symlinksAlso many EasyConfigs use the parameter
keepsymlinks
which is also set toFalse
by default and often forgotten. E.g. for the EasyBlocksMakeCp
andTarball
I propose to switch the default to copy symlinks as symlinks instead and not resolve them. This has the following benefits:
Tarball
I'd say the expectation is that the result is the same as when directly extracting the tarball to the destinationAs an alternative and extension to https://github.com/easybuilders/easybuild-framework/pull/3784 I'd propose that if the directory tree to be copied contains symlinks, then an explicit choice must be made. I.e.
keepsymlinks
defaults toNone
copy_dir
takessymlinks=None
copy_dir
symlinks
isNone
an error is thrownThis at least avoids mistakes by forgetting about that param and won't affect most installations.