Symbolic links created as outputs are not stat'ed correctly #1186
Comments
Before this change, symlinks were always followed when retrieving mtimes. This can cause ninja to think targets are stale when outputs are symlinks. This patch introduces the following stat() logic for symlinks: 1. When Stat()ing inputs, use the newer of the symlink or what it points to. * By looking at both, it maintains the pre-existing behaviour, where you could use a symlink as an input and ninja would know to rebuild when the thing it points to changes. 2. When Stat()ing outputs, always use the timestamp of the symlink. * If it's listed as an output, then it contains the only mtime ninja should care about. Fixes Bug ninja-build#1186
Rather than trying to symlink as an action after producing the bundle, do it as an early-stage action. As part of this, mac_framework_bundle callers need to explicitly specify the top-level symlinks (framework_contents) that should be created. Frameworks are now packaged like so: 1. At `gn gen` time, an exec_script runs to write the framework_version to a file. If the previous value written does not match the new value, the entire framework output directory is clobbered. This must be done at gen-time to ensure nothing tries to clean the framework while also attempting to copy to it. 2. Also at `gn gen` time, a TOC file for the framework_contents is written. This allows the build to emulate depending on the presence of the top-level symlinks, since ninja does not stat symlinks correctly. ninja-build/ninja#1186 3. The package_framework.py action now runs before the main create_bundle(). This action depends on the TOC file from (2) and will create all the required symlinks in the bundle. At the time this runs, the symlink target may not yet exist. 4. The create_bundle target is now always the leaf edge for mac_framework_bundle, and it copies all bundle contents to the fully versioned path (rather than through a symlink). This should resolve the issue of the build not stabilizing between specifying a framework_version and not. BUG=648757 Review-Url: https://codereview.chromium.org/2487763002 Cr-Commit-Position: refs/heads/master@{#430778}
Before this change, symlinks were always followed when retrieving mtimes. This can cause ninja to think targets are stale when outputs are symlinks. This patch introduces the following stat() logic for symlinks: 1. When Stat()ing inputs, use the newer of the symlink or what it points to. * By looking at both, it maintains the pre-existing behaviour, where you could use a symlink as an input and ninja would know to rebuild when the thing it points to changes. 2. When Stat()ing outputs, always use the timestamp of the symlink. * If it's listed as an output, then it contains the only mtime ninja should care about. Fixes Bug ninja-build#1186
Before this change, symlinks were always followed when retrieving mtimes. This can cause ninja to think targets are stale when outputs are symlinks. This patch introduces the following stat() logic for symlinks: 1. When Stat()ing inputs, use the newer of the symlink or what it points to. * By looking at both, it maintains the pre-existing behaviour, where you could use a symlink as an input and ninja would know to rebuild when the thing it points to changes. 2. When Stat()ing outputs, always use the timestamp of the symlink. * If it's listed as an output, then it contains the only mtime ninja should care about. Fixes Bug ninja-build#1186
|
Copying over from the pull request: I found a repro! Let's say you're trying to build an executable, and you also want a symlink to that executable under a different name (think e.g. clang and clang-cl). In gn, you might express this as a target that creates the symlink, and then make the actual executable link depend on the symlink target (the symlink's dest does after all not have to exist when the symlink's created). gn will then make a build rule that creates a symlink, then a stamp file that depends on the symlink for the symlink action, and then a link rule for the executable, order-only depending on the stamp, like so: With that setup: |
|
One workaround then would be to not tell ninja about the output of the symlink step, but to tell ninja that that step also creates a stampfile, and make the command |
|
Woo! Nice job finding a repro! Stamp files aren't a very good work-around, because it's pretty common in our case we're symlinking executables, and it's pretty common to want to build things by typing For an even better reason it won't work: We need the files to be included in GN's runtime_deps. |
|
Note: we've certainly been getting by for quite some time without this fixed, so there's no urgency to it. Now with a repro though, I may try to get back around to figuring out if there's a way to update the code to have it work in a deterministic way. |
|
Have also been bitten by this - have a symbolic link that is always dirty because it points to something old. Presumably the fix it to change a stat() to an lstat() somewhere? |
|
@cdyson37 There's no easy fix, see #1188 (comment) (and the rest of the discussion there); it's not clear what (if anything) should be done. #1186 (comment) has a workaround. |
This comment was marked as abuse.
This comment was marked as abuse.
In Chromium's Android build, we have a rule that creates a symlink. We mark it as an output in ninja, but it looks like ninja is using
statrather thanlstat, so it is actually following the symlink when querying its mtime rather than using the output's actual mtime.I think there are cases where you would actually want to follow the symlink when requesting its mtime. Namely - when there's a non-generated input (or basically any time where you're not actually dealing with a symlink that's an output.
The text was updated successfully, but these errors were encountered: