Commit Graph
4394 Commits
Author SHA1 Message Date
eldritch horrors 40cf413a48 libstore: make acceptRanges a function
we don't need to keep this as state. it's only used for retries anyway.

Change-Id: I807f161a516a226567972757ec07ff91b3cf0899
2024-11-09 20:08:48 +00:00
eldritch horrors 90536e27e1 libstore: simplify TransferSource::awaitData
don't pass in a lock that's only used inside this function.

Change-Id: I15c01e9cfe343cf13828ae3b3742c36ae697291f
2024-11-09 20:08:48 +00:00
eldritch horrors 5d02800e57 libstore: do not retry FileTransfer uploads
this only ever worked for empty uploads, and there it worked only by
complete accident: curl was asked to send more data than the wrapper
would provide, which curl would not like and report as an error. the
error would cause a retry with even less data to send, until finally
failing by running into the retry limit. let's just forbid all this.

Change-Id: I229a94b3b8b33e2c6cdb8ea19edd57cd6740e6c6
2024-11-09 20:08:48 +00:00
eldritch horrors fb85228755 libstore: extract eager transfers into new method
we'll need this shared code for kjified transfers that don't use
sources. it's a while out, but we can clean this up now already.

Change-Id: Ife8c160e6ab379761362d6c54aba05093deee99e
2024-11-09 20:08:48 +00:00
eldritch horrors 12156d3beb libstore: fix download thread notifications
since 4ae6fb5a8f dropping a source of a
download might not properly cancel the associated curl transfer after
the transfer was paused. we have also not unpaused the transfer often
enough, only if the transfer buffer had been drained in its entirety.

Change-Id: Ic9298d9df71daa0f3d1c3fd718ed441edae9e863
2024-11-09 20:08:48 +00:00
Lily Ballard 3c8096e5cb feat: Add temp-dir setting
This adds a new temp-dir setting for controlling the temporary directory
without having to change the TMPDIR env var. This can be used to e.g.
use a path on a case-sensitive store on macOS for temporary files
without changing the TMPDIR var used by interactive shells or commands
invoked with `nix run`.

This also stops unsetting `TMPDIR` on darwin when the env var value
starts with `/var/folders/`, preferring instead to just do the check
when reading `TMPDIR`. This way the inherited `TMPDIR` env var is
preserved for child processes (such as interactive shells).

As a side effect this changes the behavior of `nix-build -o ''` to act
like `nix-build --no-out-link` instead of failing with an error caused
by trying to create a symlink at the cwd.

Fixes: https://git.lix.systems/lix-project/lix/issues/253
Fixes: https://git.lix.systems/lix-project/lix/issues/112
Change-Id: I9ee826323f2deca62854715a77ca7a373a948a29
2024-11-06 18:11:47 -08:00
Lily BallardandGerrit Code Review 72cce7be3f Merge "libstore: replace random() calls with atomic counter" into main 2024-11-07 00:15:23 +00:00
eldritch horrors 1c28270c9d libstore: don't hold state lock while unpausing transfers
libcurl may call callbacks during unpause. if libcurl calls these
callbacks often enough they may find that they've exhausted their
allotted buffer space, at which point they will call dataCallback
as provided in enqueueFileTransfer. download() provides callbacks
that have their own state locks and may call unpause on transfers
with *their* state locks they share with curlFileTransfer. it was
possible to cause a deadlock between these two if the curl worker
thread tried to provide some data to a callback with the transfer
state lock held and the user transfer simultaneously unpaused its
associated curl transfer, with its own state lock held. obviously
not a great situation, but avoidable by not holding any lock when
we unpause transfers and execute their callbacks, as is otherwise
the case when a transfer runs normally and is never paused at all

Change-Id: I58556d292adaf7dfb14001d3a6c5c38fa71994da
2024-11-04 23:54:41 +00:00
Lily BallardandGerrit Code Review b1a0e3c002 Merge "libstore: remove unnecessary lstats in optimisePath" into main 2024-11-03 05:15:40 +00:00
Artemis Tosini 3aad088cf1 libstore: Fix FreeBSD build
FreeBSD was left out of a few refactors over the last few months.

Add a header and register the store implementation so it's back
to working as well as it was before.

Change-Id: I6f7b2ceb557c290f2d9e0d7f207b3fea87b353ed
2024-11-02 07:23:55 +00:00
Lily Ballard 69957a971e libstore: replace random() calls with atomic counter
random() is not thread-safe, it relies on global state, and calling it
from worker threads can result in multiple threads producing the same
value. It also doesn't guarantee unique values even in single-threaded
use.

Use an atomic counter for the use-case of generating temporary paths,
and switch to a thread-local RNG for the one remaining call.

This will probably fix https://github.com/NixOS/nix/issues/7273 though
I'm not willing to risk corrupting my store to find out.

Change-Id: I4c4c4c9796613573ffefd29cc8efe3d07839facc
2024-10-30 19:54:43 -07:00
Lily Ballard 684f93e783 libstore: ignore broken symlinks in ssl-cert-file default
Also tweak `pathAccessible` to ignore other relevant errors too. It was
documented as ignoring permission errors but it was only ignoring
`EPERM`, which comes from the darwin sandbox, and not ignoring `EACCESS`
which is the real permission error. I figured it also makes sense to
ignore `ELOOP`.

Fixes: https://git.lix.systems/lix-project/lix/issues/560
Change-Id: Ibb849b68d07386eb80afb52b57f7d12b3a48a202
2024-10-30 19:50:38 -07:00
eldritch horrors beb193d1e2 libstore: remove our custom early timeout handling
curl handles timeouts internally when it acts as the main event loop. we
only need to wake up to add new transfers to the multi handle or restart
an existing transfer that has passed its restart wait time. periodically
waking up is not required for curl, and we don't need it any more either

Change-Id: Ic774e1d9519f807cda1a89694bc3ede75216f329
2024-10-30 22:52:19 +00:00
eldritch horrors 4ae6fb5a8f libstore: pause only stalling transfers
don't pause the entire curl thread. we have multiple consumer threads
after all, not just one, so stalling all of them is likely not great.

note that libcurl advises against using transfer pauses if compressed
encodings are allowed and automatically decoded. this should not lead
to problems in practice because our data is usually not compressed to
such a degree that curl buffering *uncompressed* data matters. should
this cause problems we can reintroduce the whole-thread pause, but we
will probably get away with this until the entire file transfer class
is made kj::Promise-using async (and *then* curl can be hardpaused if
it cannot get rid of its data, solving the problem once and for all).

Change-Id: I218e41bfa5a27c7454eafb0bdb54f2a29a7f6493
2024-10-30 22:52:19 +00:00
Linus HeckemannandGerrit Code Review c95b73d8a1 Merge "libstore: report all differing outputs rather than just the first" into main 2024-10-30 19:04:57 +00:00
Dusk BanksandGerrit Code Review 8b2f8d538b Merge "libstore: restore mode after changing xattrs" into main 2024-10-30 14:56:43 +00:00
Linus Heckemann 8b0ac51f12 libstore: report all differing outputs rather than just the first
Before:

error: derivation '/nix/store/4spy3nz1661zm15gkybsy1h5f36aliwx-python3.11-test-1.0.0.drv' may not be deterministic: output '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist' differs from '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist.check'

After:

error: derivation '4spy3nz1661zm15gkybsy1h5f36aliwx-python3.11-test-1.0.0.drv' may not be deterministic: outputs differ
         output differs: output '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist' differs from '/nix/store/ccqcp01zg18wp9iadzmzimqzdi3ll08d-python3.11-test-1.0.0-dist.check'
         output differs: output '/nix/store/yl59v08356i841c560alb0zmk7q16klb-python3.11-test-1.0.0' differs from '/nix/store/yl59v08356i841c560alb0zmk7q16klb-python3.11-test-1.0.0.check'

Change-Id: Ib2871fa602bf1fa9c00e2565b3a2e1b26f908152
2024-10-29 18:34:54 +01:00
eldritch horrors 9c22a4d31b libstore: don't use curl for file:// downloads
curl can't pause downloads of file:// urls, which is very much in the
way of making the curl wrapper fully asynchronous. we can emulate all
the things curl does, but unfortunately curl is *rather extensive* in
its support of frankly weird shit. hopefully this subset of features,
which notably does not include curl readdir support, is enough for us

Change-Id: I5f67768c4b512565655b94b0421270c7dbbd8d11
2024-10-28 18:52:49 +00:00
eldritch horrors c83b13eafd libstore: reunify all file transfer methods again
with the api cleaned up we can suddenly reunify uploads, downloads, and
existence checks through curl in the same wrapper function. uploads and
existence checks simply don't use the result source, and given that all
transfers (or at least *most* transfers to date) go through the network
the few extra allocations do not hurt us at all. even for file:// calls
the overhead won't much matter as going to disk and back *is* expensive

Change-Id: I4f9ca6681a8fc303377b4cf4c63e3363ae32c18b
2024-10-28 18:52:49 +00:00
eldritch horrors d65838a900 libstore: remove FileTransfer::enqueueDownload
it's no longer needed. `download` can do everything `enqueueDownload`
did, and a lot more. e.g. not block the calling thread, for instance.

Change-Id: I4b36235ed707c92d117b4c33efa3db50d26f9a84
2024-10-28 18:52:49 +00:00
eldritch horrors c68f0cdf00 libstore: return transfer metadata from download
as promised earlier. nothing uses it yet, but just you wait.

Change-Id: I77d185578d96c2134b756d20f2fcf1c02de0da6f
2024-10-28 18:52:49 +00:00
eldritch horrors 14eff10fe4 libstore: split callback into metadata and finished parts
this will let us return metadata from FileTransfer::download, which in
turn is necessary to remove enqueueDownload. it also opens avenues for
streaming downloads that keep download metadata instead of dropping it

Change-Id: If0fc6af5eb2aeb689fc866c345c9d7bce4d59f2d
2024-10-28 18:52:49 +00:00
eldritch horrors 923abe347c libstore: use data callback for simple downloads too
this is an intermediate step towards removing enqueueDownload entirely.

Change-Id: I05ec0c7f4a234fdc966e5005308b37f6f905d433
2024-10-28 18:52:49 +00:00
eldritch horrors 64864c3730 libstore: pass only data to TransferItem data callback
with encoding being handled by curl the reference is no longer needed.

Change-Id: Ibfaf5f55e5314e81ce45ba4523b960c401dd2e1c
2024-10-28 18:52:49 +00:00
eldritch horrors 10488f7431 libstore: use curl content-encoding support, not our own
let's use the automatic decoding functions curl provides instead of
implementing them ourselves for the dubious ability to support both
xz and bzip2 encodings as well, neither of which anything will send

Change-Id: I3edfebeb596a0e9d5c986efca9270501c996f2dd
2024-10-28 18:52:49 +00:00
Lily Ballard a042bbde56 libstore: remove unnecessary lstats in optimisePath
We were calling `lstat()` twice on the link path, and a third time if it
existed. We only need to call it once.

Change-Id: Ifadc2f24681648d0ec7e4bad5c6d2dcb2e560e7b
2024-10-27 20:03:29 -07:00
eldritch horrors 212a14bb1f libstore: check that transfer headers don't change during retries
etag changing implies with high probability that the content of the
resource changed. immutable url changing implies that the immutable
url we got previously was wrong, which is probably a server bug. if
the encoding changes our decoding will break completely, so that is
also very illegal. one notable change we still allow is etags going
away completely, mostly since this does not imply any data changes.

Change-Id: I0220ceddc3fd732cd1b3bb39b40021cc631baadc
2024-10-27 21:44:38 +00:00
eldritch horrors 7c716b9716 libstore: use effective transfer url for retries
do not retread the entire redirection path if we've seen the end of the
road. this avoids silently downloading wrong data, and notifies us when
a url we've received data from turns into a redirect when retrying. for
reasons of simplicity we don't turn of libcurl redirects on retries. if
we did that we'd have to conditionally process http status codes, which
sounds annoying and would make the header callback even more of a mess.

Change-Id: Ide0c0512ef9b2579350101246d654a2375541a39
2024-10-27 21:44:38 +00:00
eldritch horrors 2b3bdda027 libstore: collect effective url and cachedness earlier
this will let us return metadata for a transfer without having to wait
for the entire transfer to complete. more importantly for current uses
though is that we could now send retries to the effective url directly
instead of retreading the entire redirect path. this improves latency,
and in such cases where redirects change while we're downloading it'll
also improve correctness (previously we'd silently download bad data).

Change-Id: I6fcd920eb96fbdb2e960b73773c0b854e0300e99
2024-10-27 21:44:38 +00:00
eldritch horrors 97c76c4655 libstore: remove TransferItem::active
it's always legal to call curl_multi_remove_handle on a valid pair of
multi and easy handles. removing an easy handle that is not currently
attached to a multi handle is a no-op, and removing an easy handle of
a different multi handle is something we can't reasonably trigger. if
we *did* ever manage it would result in an error we'd ignore, and the
handles in question would not be changed at all. this is just simpler

Change-Id: I85ec62ff89385981ca49d243376b9c32586bd128
2024-10-27 21:44:38 +00:00
eldritch horrors c82407fc1e libstore: always allocate TransferItem::req
there's no reason not to do this. also improve error handling a bit.

Change-Id: I35853a919fa58a9a34ad47ffab6de77ba6f7fb86
2024-10-27 21:44:38 +00:00
eldritch horrors 982d049d3b libstore: remove FileTransferResult::data
return it as a separate item in a pair instead. this will let us remove
enqueueDownload() in favor of returning metadata from download() itself

Change-Id: I74fad2ca15f920da1eefabc950c2baa2c360f2ba
2024-10-27 21:44:38 +00:00
eldritch horrors 5cd7055044 libstore: de-future-ize FileTransfer::enqueueUpload
it's only used once, and synchronously at that.

Change-Id: Ife9db15dd97bc0de8de59a25d27f3f7afeb8791b
2024-10-27 21:44:38 +00:00
eldritch horrors 6f18e1ebde libstore: remove FileTransferRequest
it's just a uri and some headers now. those can be function arguments
with no loss of clarity. *actual* additional arguments, for example a
TLS context with additional certificates, could be added on a new and
improved FileTransfer class that carries not just a backend reference
but some real, visible context for its transfers. curl not being very
multi-threading-friendly when using multi handles will make sharing a
bit hard anyway once we drop the single global download worker thread

Change-Id: Id2112c95cbd118c6d920488f38d272d7da926460
2024-10-27 21:44:38 +00:00
eldritch horrors a839c31e6c libstore: remove FileTransferRequest::expectedETag
just another http specific used in only one place.

Change-Id: I99361a7226f4e6cd8f18170d3683c0025657bcb3
2024-10-27 21:44:38 +00:00
eldritch horrors 30bec83fa4 libstore: remove FileTransferResult::bodySize
it's only used for internal bookkeeping in TransferItem.

Change-Id: I467c5be023488be4a8a76e5f98a4ef25762df6f3
2024-10-27 21:44:38 +00:00
eldritch horrors d82b212d33 libstore: remove FileTransferRequest::verifyTLS
it's never set to false.

Change-Id: I1e436c82f1097091a08faa1dfada75e51bd5edf9
2024-10-27 21:44:38 +00:00
eldritch horrors 220251ba51 libstore: remove FiletransferRequest::head
add a method to FileTransfer that provides this functionality instead.

Change-Id: Ic1933a5df76a109c248c9c5efea065356b20a6f9
2024-10-27 21:44:38 +00:00
Raito BezariusandGerrit Code Review f9e7df01f3 Merge "daemon: stop eating SIGINTs" into main 2024-10-27 18:11:20 +00:00
eldritch horrors 6c2609c5f9 libstore: remove FileTransferRequest::tries
it's never set, and then only used internally. *once*.

Change-Id: I32585b1821e979f3ebb53b794ba0d1f576126b92
2024-10-26 21:42:35 +00:00
eldritch horrors af27d1ecd8 libstore: make baseRetryTimeMs a FileTransfer property
we don't even need this outside of tests. maybe we should not do
automatic retries at this level at all and use retrying wrappers
instead? at some point we may have to do this, but not just yet.

Change-Id: If0088aa55215be81f1770c25b3bb1b5268c65cf8
2024-10-26 21:42:35 +00:00
eldritch horrors 1e3b45546c libstore: remove FileTransferRequest::parentAct
never set explicitly, and transfers are never instantiated with one
current activity but submitted with a *different* current activity.

Change-Id: I1a3ec57c02013565aeb9e9398ea42d0c4279095e
2024-10-26 21:42:35 +00:00
eldritch horrors ce3e1d1e7a libstore: remove FileTransferRequests::data
use separate upload and download methods instead.

Change-Id: I5baa2177c8ddd70268c75ff074e361b2f17dddbd
2024-10-26 21:42:35 +00:00
eldritch horrors 2d49efaa2e libstore: remove Filetransfer::transfer
just use enqueueFileTransfer().get() insteaad.

Change-Id: I67a43c9d3d5f68ac3f9e8ba7973c243dd78b86a3
2024-10-26 21:42:35 +00:00
eldritch horrors 98b55c3a1d libstore: move FileTransferRequest::verb to TransferItem
this function is only used internally by curl wrapper.

Change-Id: I71d4c430cb069e2c949be769c17fede8dd04d480
2024-10-26 21:42:35 +00:00
eldritch horrors a83bf24281 libstore: remove FileTransferRequest::mimeType
it's only used by HttpBinaryCacheStore, and even there used in only on
place. this one place can set the header explicitly, which it now does

Change-Id: Id89228150669e25e7f59a3d6bd939e46059ce29e
2024-10-26 21:42:35 +00:00
eldritch horrors a8d6577bf0 libstore: HttpBinaryCacheStore::{makeRequest -> makeURI}
it only sets the one field anyway (and the parent activity as a side
effect that does not depend on the exact location of the constructor
call). when FileTransferRequest goes away we would need this anyway.

Change-Id: I35cf2ed3533239181449a62cf34cd282b395e5db
2024-10-26 20:35:16 +00:00
alois31andGerrit Code Review 2734a9cf94 Merge changes I29e66ad8,I77ea62cd,I7cd58d92 into main
* changes:
  treewide: make more settings conditionally available
  libstore/build: only send overridden settings to the build hook
  treewide: consistently mark overridden settings as such
2024-10-23 15:20:51 +00:00
eldritch horrors 5f1344dd8a libstore: turn Worker::run into a promise
a first little step into pushing the event loops up, up and away.
eventually we will want them to be instantiated only at the roots
of every thread (since kj binds loops to threads), but not today.

Change-Id: Ic97f1debba382a5a3f46daeaf2d6d434ee42569f
2024-10-23 11:55:12 +00:00
eldritch horrors faee771b30 libstore: hide Worker and goals where possible
goals should be considered internal to the worker architecture due to
the tight coupling of the two, and we can finally do that. doing this
is also a prerequisite for turning Worker::run() into a real promise.

Change-Id: I7cf273d4a6fdb75b8d192fce1af07c6265ff6980
2024-10-23 11:55:12 +00:00