Handling working-directory in sourcekitd

Development Compiler
#1

I want to fix a hole in sourcekitd: we don’t have a mechanism to set the working directory, so if a semantic request’s command line arguments use relative -F or -I paths, it will generally fail. Typically build systems will cd before invoking swiftc, but sourcekitd cannot do likewise. We see this fairly often in practice. In fact, it’s even worse because we may get -Xcc -working-directory among the arugments, which means the clang importer may be out-of-sync with the swift driver about the presence or location of modules from relative search paths. In the past, this has caused crashes.

I’ve thought about three ways we could deal with this.

1. Do nothing. Whoever calls sourcekitd needs to use absolute paths

The big downside I see with this model is that if the client application allows users to specify arbitrary compiler options (e.g. OTHER_SWIFT_FLAGS in Xcode), then this would only be possible if the client application knows about all the significant places paths might show up in swift command line arguments. I’d rather the swift driver be the one true source of knowledge here. It’s also not really an enforceable model unless we’re willing to explicitly disallow relative paths, which would be somewhat inconvenient for use on the command line.

2. Add a -working-directory driver option

This would work similarly to the clang -working-directory option, except we could make it a driver option and not a frontend option. This has several advantages, such as (a) it’s easy for any tool to add the working directory without understanding the rest of the command-line arguments, (b) command-line arguments are already passed through everywhere we need them and they’re well-understood, and © the driver is an ideal place to have knowledge about what could be affected by the working directory and to test this behaviour.

The disadvantage that I see with this option, is that we have to decide how it affects all filepaths in the driver, not just the ones that affect sourcekitd. For example, sourcekitd doesn’t care what happens with linker flags, but if we published this option in the driver, we would have to take a stance one way or the other I guess.

3. Pass working-directory out-of-band in sourcekitd

The driver already has some support for setting a working directory in its internal APIs, we just need to get the argument passed through. Passing it as an extra parameter to sourcekitd requests (ie. key.working_directory: "/path") would work fine, but does mean a bunch of extra plumbing. We’d need to make every layer that passes through compiler arguments aware of the extra thing, add it to logging output where we log compiler invocations, etc. It also means we need to test the behaviour at these entry points instead of just the driver itself.

One short-term advantage of this option is that adding new keys to sourcekitd requests doesn’t completely break compatibility with older toolchains (they just won’t use the working-directory), whereas passing a new driver option does break compatibility.

Thoughts? My opinion is that we should choose either (2) or (3); I have a weak preference for (2).

CC @jrose

-Ben

#2

I still don’t like the idea of a -working-directory option. It’s never quite been a sensible model to me, at least partly because it’s a process-wide setting.

That said, the frontend is equipped to support at least the sensible part of it—affecting search paths and input paths—with the workingDirectory argument to parseArgs. But the driver does not have this, which would be important for anything involving incremental builds or build records saved in the output.

I guess (2) is an okay way to go, but we have to find everything that does anything with paths and make sure they’re properly resolved. @Graydon_Hoare may also have thoughts here.

(@Anna_Zaks, this is what we were talking about at lunch.)

#3

Can you expand on what the issue is here? In the context of sourcekit it certainly wouldn’t be process-wide, which may be distracting me.

Oh, I thought this code was already in the driver rather than the frontend. My mistake. I hadn’t considered the output of incremental mode either.

#4

Sorry, you’re right. I conflated the actual working directory status (what setcwd and getcwd do), which is a process-wide setting, with whatever we’d want to do to resolve relative paths here. I guess I can’t point to any concrete reasons not to want a -working-directory option, even though I really thought I used to have some.

#5

I’m still willing to go the other way if you can think of any strong reasons not to add the flag. Barring that, I’ll take a stab at implementing -working-directory.

#6

You’ll definitely want to coordinate with Graydon, who’s moving things around in the driver at the moment related to search and input paths.

#7

I don’t like working directories either, but I think the arguments against working directories are beyond the scope of Swift. They exist and conventions have been built around them, for better and for worse.

Dave

#8

Patch: https://github.com/apple/swift/pull/14746