Commit 1e4db68
authored
refactor(locking): Make disabling locking on NFS mounts explicit (#16177)
### What does this PR try to resolve?
~~This PR introduces a `LockingStrategy` struct and `LockingMode` enum
to allow more control over the locking strategy used during
compilation.~~
This work is done in preparation to enable fine grain locking.
The motivation is to move logic to determine if we should disable
locking outside of
[`try_acquire()`](https://github.com/rust-lang/cargo/blob/0540bde82a8d70b6d49aa16894140475070f3a63/src/cargo/util/flock.rs#L359)
* See: #16155
* cc: #4282
* For reference this behavior of disabling locking on NFS was originally
added in #2615
Notes:
1. The nfs check was not removed from `try_acquire()` as the registry
cache still relies on that check.
2. In #16155 there was an
oversight which did not consider if artifact-dir was on an nfs mount but
not build-dir. This is now taken into consideration by `LockingStrategy`
in this PR.
### How to test and review this PR?
I created a local nfs mount over localhost and mounted it at
`/mnt/nfs_test`, then set `CARGO_BUILD_BUILD_DIR` to a directory in that
mount.
```
CARGO_BUILD_BUILD_DIR=/mnt/nfs_test/dummy-build CARGO_LOG=cargo::core::compiler::locking=debug lc build
0.022885439s DEBUG cargo::core::compiler::locking: NFS detected. Disabling file system locking for build-dir
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
```
after build I verified no `.cargo-lock` exists in the build-dir
```
tree /mnt/nfs_test/dummy-build
/mnt/nfs_test/dummy-build
└── debug
├── build
├── deps
│ ├── foo-ad75d8ca32b29d47
│ └── foo-ad75d8ca32b29d47.d
├── examples
└── incremental
└── foo-2vk25yx3lch9b
├── s-hciaevvrup-08c34gu-5appljltgguri514nuemj1ibu
│ ├── 1gwd5tj9uf8jpg5ioumt8go3d.o
│ ├── 6eaoidoi64c9ig6k0953bogii.o
│ ├── 7e23o2bk07r4ts7z2xhkfxxam.o
│ ├── 7gxp0h0prbuhv3t5u8dhcyjyk.o
│ ├── 9rdgyadsm9zgjys7rfpm7d5at.o
│ ├── c8w9viecm6mfutn314xg83y53.o
│ ├── dep-graph.bin
│ ├── query-cache.bin
│ └── work-products.bin
└── s-hciaevvrup-08c34gu.lock
```
Not sure if there is an easy way to test this in CI and I am unsure if
building in network filesystem mounts are a common enough usecase to
warrant adding that complexity to the test CI.
#### Other observation during testing
I noticed that `is_on_nfs_mount()` will return `false` if the directory
does not exist but will be nested with in an NFS mount. After the
directory is created by a previous cargo invocation it will begin
returning `true`. I believe this is a bug, though its probably low
priority given that this is a probably niche usecase. I did not attempt
to fix this in this PR.3 files changed
+14
-9
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| 106 | + | |
106 | 107 | | |
107 | 108 | | |
108 | 109 | | |
| |||
152 | 153 | | |
153 | 154 | | |
154 | 155 | | |
155 | | - | |
156 | | - | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
157 | 161 | | |
158 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
159 | 166 | | |
160 | 167 | | |
161 | 168 | | |
162 | 169 | | |
163 | 170 | | |
164 | | - | |
165 | | - | |
166 | 171 | | |
167 | 172 | | |
168 | 173 | | |
| |||
222 | 227 | | |
223 | 228 | | |
224 | 229 | | |
225 | | - | |
| 230 | + | |
226 | 231 | | |
227 | 232 | | |
228 | 233 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
427 | 427 | | |
428 | 428 | | |
429 | 429 | | |
430 | | - | |
| 430 | + | |
431 | 431 | | |
432 | 432 | | |
433 | 433 | | |
| |||
445 | 445 | | |
446 | 446 | | |
447 | 447 | | |
448 | | - | |
| 448 | + | |
449 | 449 | | |
450 | 450 | | |
451 | 451 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
| 45 | + | |
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| |||
0 commit comments