[SR-11390] Incremental compilation isn't triggered when static frameworks change

[keith]

Previous ID	SR-11390
Radar	None
Original Reporter	@keith
Type	Bug

Attachment: Download

Environment

Swift 5.0 in Xcode 10.3 and Swift 5.1 in Xcode 11.0 beta 7

Additional Detail from JIRA

Votes	1
Component/s	Compiler
Labels	Bug
Assignee	None
Priority	Medium

md5: 077a54e973043a117a482957880a6580

Issue Description:

If you have a module that you're compiling incremental and that module depends on a framework using `-Fpath/to/framework -framework name`, if you change the framework's binary / swiftmodule but maintain the same path, in some cases this doesn't invalidate incremental compilation. This leads to you not rebuilding a module that depends on the changed framework, and can lead to linker failures caused by symbol changes between versions of the static frameworks.

Attached is a repro case project for this. The gist of the setup is:

• There are 2 static frameworks named `framework`, they both have a type `Foo.swift`, one has an initializer that takes `a: String` and one has an initializer that takes `a: String, b: String?`

• There is a consumer that depends on the first version of this framework, and therefore depends on this symbol `_$s9framework3FooV1aACSS_tcfC`

• The consumer is compiled against the framework with this symbol passing a -F to a symlink directory that contains that framework

• The symlink is replaced now pointing the the second framework that has a different symbol

• The consumer is recompiled incrementally, and nothing happens

If you went to link the final consumer binary it would fail since it's depending on symbol the second static binary wouldn't provide.

To reproduce this run `./repro.sh`. This script will build the static frameworks, build consumer against both of them, and diff the symbols found in consumer. If it is working correctly the symbols should be different. This script also passes `-driver-show-incremental` which you will see prints the first time for the clean compile, but not the second time since nothing happens.

[belkadan]

I don't think there's much the compiler can do about this. You went behind its back by changing the filesystem in a way it can't detect. Should it check every component of the search path to see if any of them were symlinks?

[keith]

IMO this is kind of an expected use case if developers check frameworks into source control, and do a `pull` which changes the framework. My example of swapping symlinks was just a bit easier to provide.

[belkadan]

A pull that changes the framework will update paths the compiler knows it read from, and should indeed cause an incremental recompilation. Was that the original case you were seeing, though?

[keith]

So you're saying it's just because it's a symlink that this is a problem? I was thinking that pre and post `git pull` since the paths would be the same this would be the same case? We are actually seeing the symlinking case because of how our project is setup

[belkadan]

Yep, that's correct. The compiler records the paths of all the files it read from and checks to see if they changed, but it doesn't record how it got to those paths, and I believe it has to canonicalize them to avoid including the same thing two different ways in some terrible header search cases.

[keith]

Should canonicalizing the paths resolve the symlinks and therefore in this case that would change between runs? It looks like it doesn't do that right now because if during the middle of my repro script I remove the first framework it still has the issue, so it's not checking that old one

[belkadan]

Oh, interesting. That's what I was thinking, and if it isn't then maybe something else is going on.

[belkadan]

Oof, okay. It looks like the paths tracked in the swiftdeps are not canonicalized—so I was wrong about that—and the problem is that the driver is checking if the external dependencies have been modified since the build, rather than recording their original size/mtime. This is consistent with what Makefiles do, so it's defensible, but it can clearly lead to surprises like this.

brentdax (JIRA User), davidungar (JIRA User), thoughts?

[keith]

Do you know where this logic lives in general so I can try skimming through it and see if anything jumps out?

[belkadan]

You can take a look at the swiftdeps files in the build directory to see what info is and isn't recorded. The actual checking is in scheduleAdditionalJobs in Compilation.cpp.

[keith]

It definitely makes sense from that why we're hitting this. We've worked around it in our project by touching the contents of the framework when re-creating the symlink.

[swift-ci]

Comment by David Ungar (JIRA)

Are you saying that the driver is recording the mtime of the symlink directory rather than the files being used in it?

[belkadan]

The driver (or frontend) isn't recording the mtime of external dependencies at all. It's only doing that for the input source files. In today's model, external dependencies are discovered but the only thing we record is that we depend on them (like a Makefile), so all the driver has to go on is "is the mtime newer than our last build" rather than "is the mtime newer than it was last time".

[swift-ci]

Comment by David Ungar (JIRA)

But if the mtime of an external dependency is older than our last build, how would the last build not reflect the latest external dependency? I suppose that might happen if the last build were incremental and didn't happen to recompile the one file that used the external dependency. It sounds like that is the case here?

[belkadan]

No, in Keith's case he's changed the path components leading to the dependency, meaning it's now a different file underneath. This could also happen if someone modified a file without modifying the mtime (say, by reverting it back to an earlier filesystem state, or replacing it with an mtime-preserved tar or rsync).

You could also make a very quick modification on a filesystem with low-resolution mtimes, but that's unlikely enough that it may not be worth writing code to handle.