Fixes and improvements #115
Conversation
All tests for private repositories have been removed. They were broken due to account rot, which also shows their fundamentally flawed nature. A more proper solution would be to eventually spin up local servers in a VM test.
piegamesde
force-pushed
the
dev
branch
3 times, most recently
from
430543f to
275e200
Compare
Structopt has mostly been assimilated into clap_derive, so it didn't even require much changes
Spent enough time debugging silly CppNix regressions, I'm done with that
It's ded
It's not the world, but just knowing what happens when npins shells out is pretty valuable already.
| @@ -104,7 +104,7 @@ let | |||
| npins | |||
| python3 | |||
| netcat | |||
| nix | |||
| lix | |||
There was a problem hiding this comment.
I'm fine with using lix for this but we should support running tests against nix as well. I don't think we have to default to brokenNix (better name than CppNix IMO) but there might be users and perhaps at some point we can add a smoke test as even lix might introduce bugs at some point.
There was a problem hiding this comment.
Not sure how to best do this. npins has its nix dependency fixed in the derivation because of wrapProgram $out/bin/npins --prefix PATH : "${runtimePath}", so even if the test fixture is flexible it still doesn't change what nix implementation npins uses for prefetching. Maybe we should just remove that and make it a "classic" run-time dependencey?
There was a problem hiding this comment.
Maybe, I think in the past that might have helped keep the nix biniaries compatible with what we test against but ideally we'd use the fetching code of tvix anyway (or implement it on our own).
| @@ -638,6 +639,10 @@ async fn fetch_remote(args: &[&str]) -> Result<Vec<RemoteInfo>> { | |||
| .unwrap_or_else(|| "None".into()) | |||
| ); | |||
| } | |||
| log::debug!("git ls-remote stdout:"); | |||
| String::from_utf8_lossy(&process.stdout) | |||
There was a problem hiding this comment.
While this probably works it wouldn't help a lot of git is stuck at some point during the execution. Since this is already async we could log lines as they appear? Probably something for the future but I'm usually deeply frustrated by applications that eat the error output (we only provide stdout) and that block until the subprocess is finished. Usually I just attach strace and check again, but nobody should be required to resort to that.
There was a problem hiding this comment.
I can look into it in a follow up. Ideally we'd factor out the process of shelling out into a helper function which does the appropriate wrapping, so that id doesn't have to be repeated at every call site (even though currently there are only two). In the case of git specifically, we could maybe also look into using libgit or any other Rust git bindings to call the code directly
No description provided.