Commit graph

16422 commits

Author SHA1 Message Date
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
Justin ! 8c567c0424 Merge "libutil: implement PathsSetting<PathSet>" into main 2024-10-28 14:22:59 +00:00
eldritch horrors 61146c73ce Merge changes I0220cedd,Ide0c0512,I6fcd920e,I85ec62ff,I35853a91, ... into main
* changes:
  libstore: check that transfer headers don't change during retries
  libstore: use effective transfer url for retries
  libstore: collect effective url and cachedness earlier
  libstore: remove TransferItem::active
  libstore: always allocate TransferItem::req
  libstore: remove FileTransferResult::data
  libstore: de-future-ize FileTransfer::enqueueUpload
  libstore: remove FileTransferRequest
  libstore: remove FileTransferRequest::expectedETag
  libstore: remove FileTransferResult::bodySize
  libstore: remove FileTransferRequest::verifyTLS
  libstore: remove FiletransferRequest::head
2024-10-28 01:36:45 +00:00
Linus Heckemann f55ed83991 Merge "libutil: handle json builder log messages with unexpected format" into main 2024-10-27 22:54:11 +00: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 9f682204b5 Merge changes I85b6075a,Iee41b055 into main
* changes:
  libstore: ban unpacking case hacked filenames from NARs
  testsuite: add a NAR generator with some evil NARs
2024-10-27 18:12:18 +00:00
raito f9e7df01f3 Merge "daemon: stop eating SIGINTs" into main 2024-10-27 18:11:20 +00:00
raito f7edee7c14 Merge changes I8e11ddbe,Idb8d9a00 into main
* changes:
  nix-shell: stop using dynamic format strings!!
  tests: move nix-shell related tests to subdir
2024-10-27 18:10:17 +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
eldritch horrors 59e364c2a8 tests: stop using OCR for nix copy tests
this is fragile, slow as fuck, breaks constantly under high concurrency,
and completely unnecessary since ssh bypasses the stdio file descriptors
*anyway*. we do still check that we see ssh messages to ensure that none
of our subprocess handling messes with ssh's /dev/tty, but that's it now

Change-Id: Ib8e31e1999f813d07a27efc63a9d3454a9e4fcdd
2024-10-26 17:50:54 +00:00
Yureka b020d1fc27 Merge "fix build for 32-bit platforms" into main 2024-10-26 15:46:05 +00:00
alois31 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
eldritch horrors b8cc54df0a libstore: return relevant store path in goal result
we now do not need the goal pointers any more to process worker results.

Change-Id: I1a021b862ca666bcd23fee9f38973e90e6f94a72
2024-10-23 11:55:12 +00:00
eldritch horrors 67f1aafd61 libstore: restrict curl protocols
previously it was possible to fetchurl a dict server, or an ldap server,
or an imap server. this is a bit of a problem, both because rare schemes
may not be available on all systems, and because some schemes (e.g. scp)
are inherently insecure in potentially surprising ways we needn't allow.

Change-Id: I18fc567c6f58c3221b5ea8ce927f4da780057828
2024-10-23 11:32:14 +00:00
eldritch horrors 1d9d40b2a6 libstore: move failingExitStatus into worker results
we already have a results type for the entire worker call, we may as
well put this bit of info in there. we do keep the assumption of the
old code that the field will only be read if some goals have failed;
fixing that is a very different mess, and not immediately necessary.

Change-Id: If3fc32649dcd88e1987cdd1758c6c5743e3b35ac
2024-10-22 15:32:36 +00:00
eldritch horrors 343aca3a27 libstore: clear derivation build activities when done
without this derivations do not show as completely processed in the
internal-json logs (or the newer multiline output). the former also
breaks external tools like nix-output-monitor which, like multiline
output, grow vertically until at least some goals are finally freed

Change-Id: I55758daf526ba29ae15fb82e0d88da8afb45bf5c
2024-10-22 14:55:55 +00:00
Lily Ballard 5ce1d8463a Merge "libstore: add missing #include on darwin" into main 2024-10-22 02:05:44 +00:00
Yureka b77687945e fix build for 32-bit platforms
Change-Id: I113e131eb5c66c42c0bbc60181a7faafc02ca02e
2024-10-21 08:52:29 +02:00
Lily Ballard 4308ec1ae4 gitignore: ignore *.pyc files
Running our installcheck test suite creates these files.

Change-Id: I97ac8f1aa165a491c55dff6b48486db17b75443b
2024-10-20 22:08:35 -07:00
Lily Ballard 65551175e3 libstore: add missing #include on darwin
optimise-store.cc used std::regex on darwin, but forgot to include the
header. This probably compiled due to the precompiled headers file, but
it caused errors in the editor.

Change-Id: I23297c08cb66d44e4d4f303560f46e4adc7d5a43
2024-10-20 22:08:35 -07:00
Lily Ballard 068f4b147d libstore: fix sign comparison warnings in darwin platform
It's not clear to me if `proc_pidinfo()` or `proc_pidfdinfo()` can
actually return negative values, the syscall wrappers convert `-1` into
zero and the semantics suggest that negative values don't make sense,
but just to be safe we'll preserve the int type until we've checked that
it's a positive value.

Fixes: #548
Change-Id: If575aec6b1e27dba63091c7a0316c7b3788747cd
2024-10-20 13:13:11 -07:00
eldritch horrors 0ff8f91325 libutil: disallow AsyncCollect relocations
some promises capture `this`.  we could also allocate a shared state,
but this thing doesn't really need to ever be moved anyway. so there.

Change-Id: I50b5c44684a8ab4e984b1323de21f97ace4a864a
2024-10-19 19:47:46 +00:00
eldritch horrors b0e619b8bd Merge "libstore: always release build/substitution slot tokens" into main 2024-10-19 18:32:30 +00:00
eldritch horrors 564d931134 libstore: always release build/substitution slot tokens
not doing this can freeze slots until the goal that occupied them is
freed (rather than simply complete), and then can freeze the system.

fixes #549

Change-Id: I042df04222f8ffbaa18ef4a4eae6cbd6f89b679e
2024-10-19 16:17:58 +02:00