dependencies are not strict

[mpalczew]

A.swift

public class A {
    public init() {}
}

B.swift

import A

public class B {
    let a: A = .init()
    public init() {}
}

C.swift

import B // Should be an error B is not in transitive closure of deps

class C {
    let b: B = .init()
}

BUILD

load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library")

swift_library(
    name = "A",
    srcs = ["A.swift"],
)

swift_library(
    name = "B",
    srcs = ["B.swift"],
    deps = [":A"],
)

swift_library(
    name = "C",
    srcs = ["C.swift"],
    deps = [":A"],
)

bazel clean
bazel build :C  # fails
bazel build :B 
bazel build :C # succeeds should fail

[kastiglione]

fwiw a workaround is to have one library per package/directory.

[keith]

To verify, this fails even with sand boxing enabled?

[thomasvl]

Also, exactly what flags do you have enabled on the build? Isn't the a feature for a side module cache, do you have that enabled and thus you might have enabled this directly?

[allevato]

Yeah, as @keith mentioned, this sounds like you're building without sandboxing enabled. All libraries in the same package emit their .swiftmodules to the same bazel-bin subdirectory, and that subdirectory will be passed to swiftc as a search path, so the second build of :C will see B.swiftmodule even though it's not an input to the compile action.

We could make this less likely be emitting each .swiftmodule into its own target-specific subdirectory, but that would increase the overall number of search paths passed to the compiler, which would increase the number of unnecessary filesystem accesses that the compiler makes to find modules. So there would be trade-offs, and likely a performance regression.

This isn't a Swift-specific issue; as another example, without sandboxing, an Objective-C source file could import headers from anywhere in the workspace, including headers that aren't in the deps of the target being compiled.

[kastiglione]

but that would increase the overall number of search paths passed to the compiler

Correct me if I'm wrong, but this is only in cases of more than one swift_library per package. Otherwise, I believe the number of search paths would by the same if there's a 1:1 library per package.

[allevato]

Correct me if I'm wrong, but this is only in cases of more than one swift_library per package.

Right, which is the case the user has above, and would be the general case since there is no requirement that a package only contains a single swift_library.

To flip it around, if you only have one swift_library per package, you wouldn't notice the performance regression because you've already done it to yourself. 🙂

[kastiglione]

You're not getting me that easily, the swift compiler did it to me.

[kastiglione]

I'll say this though, our experience with bazel is that, time after time, correctness gets chosen over performance. I say this not to advocate one thing or another, but just to gripe 😄

[mpalczew]

To verify, this fails even with sand boxing enabled?

I believe so. How do I make sure sandboxing is enabled?

My bazelrc

build --apple_platform_type=ios

Also tried with and without

build --strategy=SwiftCompile=standalone

Same result.

It does bother me that the output of the build results appears to be non-deterministic.

e.g. bazel build ... or bazel test ... may or may not fail depending on what ends up getting executed first which may depend on remote cachine and how many cores are working at a time. i.e. might work on a 1 core machine, and fail on a 16 core machine.

[allevato]

You can try --strategy=SwiftCompile=sandboxed to force it to use the sandbox, to see if that prints out any messages about the sandbox not being enabled somehow.

...which makes me wonder now, since SwiftCompile also supports workers for incremental compilation, the default strategy for that action is actually remote,worker,sandboxed,local, meaning that the worker will be preferred over sandboxed execution. And worker execution is specifically not sandboxed (the default value for --worker_sandboxing is false) so that it can non-hermetically store incremental state outside of the regular output locations in order to provide that incrementality. So that might actually be what's going on here.

What's not clear is how to resolve it; there's a --sandbox_writable_path flag that you can use to pass additional paths that the sandbox should let you write into, but the path where the incremental state is stored is also under blaze-out/<configuration>/bin/_swift_incremental, so it's not something that can be easily determined a priori to pass on the command line (or add to .bazelrc).

So for now, I think you'll have to explicitly force the sandboxed strategy, which means you'll lose swiftc -incremental mode, which may or may not matter to you. If you're strictly concerned about determinism, then you probably don't want incremental mode, since the Swift compiler does write timestamps into the .swiftdeps files that it generates.

[mpalczew]

Just to circle back here and provide more context.

Sandboxed does build what I need it to build and behaves appropriately.
However due to bugs like https://github.com/bazelbuild/rules_swift/issues/310 And using the workaround, sandboxed is the only way I can build.

Otherwise it will add duplicate modules to the path in some circumstances.

[mpalczew]

ok, found another problem.

When using remote caching, and compiling non sandboxed, and then compiling sandboxed. The nonsandboxed builds fine, gets cached and then the sandboxed build will use the version that was cached, which really should have never compiled.

[tinder-maxwellelliott]

Just to circle back here and provide more context.

Sandboxed does build what I need it to build and behaves appropriately. However due to bugs like #310 And using the workaround, sandboxed is the only way I can build.

Otherwise it will add duplicate modules to the path in some circumstances.

I am also experiencing situations where I am getting duplicate modules, did adding --strategy=SwiftCompile=sandboxed stop this from happening to you?

[keith]

On the original issue here I think the way out here is to support the new explicit module compilation mode, which will likely eliminate the pollution and the duplicate module issue