Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-11760] SwiftPM builds inside Docker do not re-use existing build artifacts #4651

Closed
swift-ci opened this issue Nov 11, 2019 · 31 comments
Closed
Labels

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-11760
Radar rdar://problem/73985925
Original Reporter MrMage (JIRA User)
Type Bug

Attachment: Download

Environment

Swift 4.2 all the way till Swift 5.2.4.

Additional Detail from JIRA
Votes 19
Component/s Package Manager
Labels Bug
Assignee None
Priority Medium

md5: 6c4efa086896ce15ed52fa119adc9b54

Issue Description:

See https://forums.swift.org/t/incremental-builds-with-swift-package-manager-and-linux-docker/26538 for a full description. @aciidb0mb3r has asked to file a Jira ticket about this issue.

My suspicion is that SwiftPM for some reason considers build artifacts in lower Docker layers stale and thinks that it needs to rebuild them.

@beccadax
Copy link
Contributor

@aciidb0mb3r

@swift-ci
Copy link
Contributor Author

Comment by Eric Stern (JIRA)

As a small (or maybe not so small) addition - support for something along the lines of `swift build --dependencies-only` would be hugely beneficial inside of Dockerized builds once this is resolved, in order to maximize the effect of layer caching.

It's quite clumsy and counterintuitive to make "dummy files" (as shown in the linked discussion) just to overwrite them with actual source code later.

@swift-ci
Copy link
Contributor Author

Comment by Daniel Alm (JIRA)

Fully agree with Firehed (JIRA User). Creating the dummy files is only a workaround; having an option to build only the dependencies would be a much better solution in the long term.

@swift-ci
Copy link
Contributor Author

Comment by Daniel Alm (JIRA)

I have investigated a bit more and found the following:

  1. This seems to affect only non-Swift dependencies, e.g. C and C++ ones. This is still significant, however, as SwiftNIO packages a full copy BoringSSL, which takes long to compile.

  2. The output of stat indeed changes when run in a different Docker layer; in particular, fractional timestamp values are removed. Example stat output from the layer the compilation has been run in:

  File: .build/debug/CNIOBoringSSL.build/module.modulemap
  Size: 163         Blocks: 8          IO Block: 4096   regular file
Device: 801h/2049d  Inode: 426531      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:29.897272000 +0000
Modify: 2019-11-19 11:13:29.517272000 +0000
Change: 2019-11-19 11:13:29.517272000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.d
  Size: 4546        Blocks: 16         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 427653      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:48.345814000 +0000
Modify: 2019-11-19 11:13:48.335814000 +0000
Change: 2019-11-19 11:13:48.335814000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.o
  Size: 8664        Blocks: 24         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 427635      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:48.285814000 +0000
Modify: 2019-11-19 11:13:48.335814000 +0000
Change: 2019-11-19 11:13:48.335814000 +0000
 Birth: -

And from a "later" layer:

  File: .build/debug/CNIOBoringSSL.build/module.modulemap
  Size: 163         Blocks: 8          IO Block: 4096   regular file
Device: 801h/2049d  Inode: 2930655     Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:29.000000000 +0000
Modify: 2019-11-19 11:13:29.000000000 +0000
Change: 2019-11-19 11:14:10.058600000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.d
  Size: 4546        Blocks: 16         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 2930545     Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:48.000000000 +0000
Modify: 2019-11-19 11:13:48.000000000 +0000
Change: 2019-11-19 11:14:09.948600000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.o
  Size: 8664        Blocks: 24         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 2930546     Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:13:48.000000000 +0000
Modify: 2019-11-19 11:13:48.000000000 +0000
Change: 2019-11-19 11:14:09.948600000 +0000
 Birth: -

Unfortunately, even packaging the build archive in a POSIX tar which is supposed to fully preserves timestamps yields different timestamps, as well as causing a rebuild of all non-Swift dependencies:

  File: .build/debug/CNIOBoringSSL.build/module.modulemap
  Size: 163         Blocks: 8          IO Block: 4096   regular file
Device: 801h/2049d  Inode: 428027      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:14:15.888600000 +0000
Modify: 2019-11-19 11:13:29.517272000 +0000
Change: 2019-11-19 11:14:15.888600000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.d
  Size: 4546        Blocks: 16         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 696234      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:14:15.958600000 +0000
Modify: 2019-11-19 11:13:48.335814000 +0000
Change: 2019-11-19 11:14:15.958600000 +0000
 Birth: -
  File: .build/debug/CNIOBoringSSL.build/crypto/crypto.c.o
  Size: 8664        Blocks: 24         IO Block: 4096   regular file
Device: 801h/2049d  Inode: 697210      Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2019-11-19 11:14:16.008600000 +0000
Modify: 2019-11-19 11:13:48.335814000 +0000
Change: 2019-11-19 11:14:16.008600000 +0000
 Birth: -
  1. I have uploaded a full build log to https://gist.github.com/MrMage/943f0de9009ee183ae66591eafad6c7a and a sample project to https://github.com/MrMage/docker-caching-test. The Dockerfile is a bit messy with all these experiments, but I think you can see what I had tried to somehow force Swift to use the build cache.

@aciidb0mb3r I would really appreciate if you could take a look. Thanks!

@ankitspd
Copy link
Member

Thanks for the investigation! If the timestamp is changing then I suspect we'll have to use file hashing to determine if the manifest changed.

@swift-ci
Copy link
Contributor Author

Comment by Daniel Alm (JIRA)

@aciidb0mb3r another option might be to use only the integer part of the timestamp, dropping the fractional part.

@swift-ci
Copy link
Contributor Author

Comment by Pierpaolo Frasa (JIRA)

Is there any update on this issue?

@swift-ci
Copy link
Contributor Author

swift-ci commented Feb 2, 2020

Comment by Will Lisac (JIRA)

Something like `swift build --dependencies-only` would be a great addition.

I'm also using the dummy files technique and it becomes increasingly tedious as the number of targets grows.

@swift-ci
Copy link
Contributor Author

Comment by Bridger Maxwell (JIRA)

Would it be appropriate to round the timestamp here in the Swift Package Manager source?

let modTime = attrs[.modificationDate] as? Date

@swift-ci
Copy link
Contributor Author

Comment by Daniel Alm (JIRA)

This might help, provided it also gains support for Linux: #2774

@drewcrawford
Copy link
Contributor

I think one cause of detecting artifacts as "stale" is that llbuild looks at the inode number. There is an open PR to change this algorithm apple/swift-llbuild#641

I believe that separately, there is a way to adjust llbuild behavior by supplying a key to the right location. According to the documentation

> A file-system field may be supplied that toggles how the build system handles file system use. The default mode uses detailed stat information for detecting file changes. The device-agnostic mode will ignore device and inode values.

@swift-ci
Copy link
Contributor Author

Comment by Bridger Maxwell (JIRA)

The blog post for Swift 5.3 mentions better caching https://swift.org/blog/swift-5-3-released :

Faster incremental build times by avoiding duplicated compiler work across source files, and more accurately identifying code that has not changed from the previous build

I can't find any other information about this change. Any chance it relates to this bug? Lately I'm desperate for something to speed up my Swift builds.

@swift-ci
Copy link
Contributor Author

Comment by Daniel R (JIRA)

+1 on something like `swift build --dependencies-only`

I'm also creating dummy files as a workaround.

@weissi
Copy link
Member

weissi commented Feb 3, 2021

@swift-ci create

@weissi
Copy link
Member

weissi commented Feb 12, 2021

My colleague Euan Harris just made me aware of a new (experimental) Docker feature in BuildKit which can alleviate this problem. If you create your Dockerfile like so

# syntax = docker/dockerfile:1.2
FROM 5.3-focal AS builder
ADD . /code/
WORKDIR /code
RUN --mount=type=cache,target=/code/.build swift build -c release && cp /code/.build/release/my-binary /app

FROM swift:5.3-focal-slim AS runner
COPY --from=builder /app /app
ENTRYPOINT /app

then it will rebuild fast. You will need to use docker buildx build . and not docker build .. (Partial) docs about BuildKit https://docs.docker.com/buildx/working-with-buildx/ and https://docs.docker.com/develop/develop-images/build_enhancements/ .

As an explainer, the --mount=type=cache,target=/code/.build will create a cache volume that is persisted to /code/.build. It keeps the inode numbers as well as the timestamps intact and therefore SwiftPM will not rebuild unnecessarily.

Long story short, if you can, use BuildKit.

If for whatever reason you cannot use BuildKit, you can contact me directly, I have a hack that lets you use cached builds with plain old stuff (no BuildKit).

@swift-ci
Copy link
Contributor Author

Comment by ML (JIRA)

I ran into the same problem with aws codebuild which uses a buildspec.yml for the build commands. @weissi would you be so kind and share your "hack" as buildkit is not suitable in codebuild(as far as I know).

Thanks!

@swift-ci
Copy link
Contributor Author

Comment by spk (JIRA)

@weissi using CircleCI here. Could you please share your "hack" ?

Thanks!

@weissi
Copy link
Member

weissi commented Feb 21, 2022

spk (JIRA User) Sure, I used this C file that I compiled into a dynamic library (.so) and then loaded using LD_PRELOAD

@weissi
Copy link
Member

weissi commented Feb 21, 2022

Oh, with "this file" I mean the now attached override-stat.c. This lies to the SwiftPM and just returns a chopped of mtime (just the seconds, not the nano seconds) and makes the inode number a function of the sip-hash'd file name instead of a more or less random number. That makes SwiftPM trust that the files from Docker indeed haven't changed (despite the fact that both their inode numbers and mtime (missing nanoseconds)) have changed.

Again, the right solution is to use this docker feature:

RUN --mount=type=cache,target=/code/.build swift build -c release

which instructs docker to mount a volume instead of copying in the files. That'll make everything work without hacks.

@swift-ci
Copy link
Contributor Author

Comment by spk (JIRA)

Thanks so much! 🙂

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
@Terky
Copy link

Terky commented Jul 2, 2022

@weissi, it looks like the hack you provided in the original JIRA ticket is gone. Could you please attach that file again?

Also is there any chance this issue will be resolved without making a switch to the BuildKit?

@weissi
Copy link
Member

weissi commented Jul 4, 2022

@Terky there you go. You'd need to compile override-stat.c as a shared library and then LD_PRELOAD it with every swift build command you run...

But really, you should switch to BuildKit with cache layers. BuildKit is now enabled by default in docker build, any particular reason you can't adopt that?

override-stat.tar.gz

@danielBreitlauch
Copy link

BuildKit with cache mount is still no full solution:
Github Actions spawn a new node with every run. The BuildKit cache folder will be deleted. It is possible to hack/cache that folder and restoring it on the new runner. But then the files have new Inodes.

IMHO there is no way around detecting code changes without Inode checks.

@weissi
Copy link
Member

weissi commented Sep 22, 2022

IMHO there is no way around detecting code changes without Inode checks.

Yes, alas, I think this is true at the moment. I couldn't immediately find a way to turn off the inode number comparisons (https://github.com/apple/swift-llbuild/blob/b7b4c5eaa798708d6233ca07908a7cab75620040/products/llbuildSwift/BuildValue.swift#L59). @dmbryson any thoughts?

@weissi
Copy link
Member

weissi commented Sep 22, 2022

@tomerd do you think you would consider a --ignore-inode-number-changes or similar option to swift build which just leaves out the inode numbers?

@neonichu
Copy link
Member

We have rdar://96403757 (SwiftPM - consider setting file-system: device-agnostic in build manifest) which may resolve this? I'm not sure we even need an option, I think passing device-agnostic in the llbuild manifest might be a reasonable thing to do always.

@doozMen
Copy link

doozMen commented Aug 29, 2023

I still have this issue. Is there an approach to build it. Is the option to ignore the inode numbers added? Could somebody with more knowledge then me create some documentation on recommended setup of a swiftpm build for CI? Or does the latter exist already?

pusukuri added a commit that referenced this issue Nov 14, 2023
sets file-system as device-agnostic in build manifest  

### Motivation:

This is the default mode in XCBuild, and allows incremental builds to
work across reboots on APFS. It would also likely address
#4651.


### Modifications:

sets file-system as device-agnostic in build manifest

### Result:

Restarting your system (or inode changes) doesn't invoke unnecessary
builds

Related to rdar://96403757.
MaxDesiatov pushed a commit that referenced this issue Dec 12, 2023
sets file-system as device-agnostic in build manifest  

### Motivation:

This is the default mode in XCBuild, and allows incremental builds to
work across reboots on APFS. It would also likely address
#4651.


### Modifications:

sets file-system as device-agnostic in build manifest

### Result:

Restarting your system (or inode changes) doesn't invoke unnecessary
builds

Related to rdar://96403757.
MaxDesiatov added a commit that referenced this issue Dec 13, 2023
Cherry-pick of #7052.

### Motivation:

This is the default mode in XCBuild, and allows incremental builds to
work across reboots on APFS. It would also likely address
#4651.

### Modifications:

Set `file-system` as `device-agnostic` in the generated build manifest

### Result:

Restarting your system (or inode changes) doesn't invoke unnecessary builds

Related to rdar://96403757.

Co-authored-by: Kishore Pusukuri <kishoreguptaos@gmail.com>
@MaxDesiatov
Copy link
Member

@doozMen is this issue reproducible for you with latest 5.10 development snapshots?

@doozMen
Copy link

doozMen commented Feb 21, 2024

@doozMen is this issue reproducible for you with latest 5.10 development snapshots?

Hmm sorry I had a look at your fix in #7189 and that seams like the valid thing to do to solve the issue. But installing a development snapshot to verify this seams like something I do not have the time for right now. I would love to test it with 5.10 do.

@MaxDesiatov
Copy link
Member

The PR linked should address the issue in 5.10 and main development snapshots. Feel free to reopen if that's not the case.

@therickys93
Copy link

Goodmorning to everybody,
maybe this is not related to the same issue, if so please ignore it.
I have a simple project using Vapor and I configured the cache in gitlab runner in order to preserve the build between different stages. I use swift 5.10 release and it caches correctly the package download, but it recompiles again all the dependencies.
In the discussion that you were doing here, is it something that you already considered or is this new to you?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests