[wip] Attempting to give more control over the behavior of symlinks when co…#667
[wip] Attempting to give more control over the behavior of symlinks when co…#667mkujawa wants to merge 7 commits intofastbuild:devfrom
Conversation
|
|
||
| /*static*/ FileIO::SymlinkBehavior FileIO::ParseSymlinkBehavior( const AString& value ) | ||
| { | ||
| if ( value.IsEmpty() || value.CompareI( "FollowSrcAndDest" ) ) |
There was a problem hiding this comment.
This is where to change the default behavior of CopyFile in bff code. If you want it to match the new way and not the normal way, you'd have IsEmpty return FOLLOW_NEITHER
There was a problem hiding this comment.
Also welcome any feedback on these names. Are there other examples of reflected "enums"?
|
|
||
| // CopyFileEx only supports FOLLOW_SRC_AND_DST and FOLLOW_NEITHER | ||
| // based on the COPY_FILE_COPY_SYMLINK flag. | ||
| // (and COPY_FILE_COPY_SYMLINK isn't really FOLLOW_NEITHER) |
There was a problem hiding this comment.
The documentation is that the new link will point to the same file as the old link, which leads me to believe that even if you make a relative symlink, COPY_LINK isn't going to produce a link relative to the new path. But I haven't actually tested that assumption
| case FOLLOW_SRC: | ||
| flags |= COPYFILE_NOFOLLOW_DST; | ||
| } | ||
| bool result = ( copyfile( srcFileName, dstFileName, nullptr, flags ) == 0 ); |
There was a problem hiding this comment.
I'm not able to test OSX
| return true; | ||
| } | ||
| // symlink will not clobber. | ||
| if ( errno == EEXIST ) |
There was a problem hiding this comment.
I think this EEXIST test is probably a needed fix even without the rest of this PR
| { | ||
| close( source ); | ||
| return false; | ||
| if ( ( errno == ELOOP && noDstFollow ) && |
There was a problem hiding this comment.
This probably only comes up when changing link behavior in a CopyFile call, like will happen for me when I go back to the old behavior in my rules
| REFLECT( m_Source, "Source", MetaFile() ) | ||
| REFLECT( m_Dest, "Dest", MetaPath() ) | ||
| REFLECT_ARRAY( m_PreBuildDependencyNames, "PreBuildDependencies", MetaOptional() + MetaFile() + MetaAllowNonFile() ) | ||
| REFLECT( m_SymlinkBehavior, "SymlinkBehavior", MetaOptional() ) |
|
https://travis-ci.com/fastbuild/fastbuild/jobs/262587667 is failing because of a bug in the 17134 Windows 10 headers that CI uses. Non-CI does not have this issue as 17763 has fixed this (changed |
2 participants
This PR supercedes #628
I have an sdk with libfoo.so, and libfoo.so.10, with libfoo.so as a symlink to libfoo.so.10. My build rules from 0.94 had a CopyFile from libfoo.so to a distribution folder which housed all the files needed to run the build result. This now just creates a broken link in that distribution folder instead of a usable .so file. From the point of view of my build, the current behavior is buggy.
My previous attempt to fix this had numerous flaws, not the least of which is that the build can't run twice because it can't overwrite the link (I think this is a problem with the current implementation as well! So, another reason why the current behavior is buggy?)
I've attempted to make good on our discussion in the previous PR, and have added some options to CopyFile. There's still some work to do to finish this up (documentation, more testing), but I wanted to get your thoughts on it before working on it more.
In this PR, I've set the default behavior back to the 0.94 behavior, which is the same as the behavior of
copyorcp, as I think those define what will be "least surprise".