-
Notifications
You must be signed in to change notification settings - Fork 1
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
update #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For being consistent with `GitDatabase`.
`orig_url` was unclear. Whose origin? `remote_url` is way more self-explanatory under a `git fetch` context.
The explanation is moved to be public from an inline comment.
Although this is only a few lines of code, I think it is still worth keeping it clean if it's not used by Cargo internally. Not to mention this apparently has an alternative solution at this moment, for example: ```diff - let oid = git_remote.rev_for(db_path, git_ref)?; + let oid = git_remote.db_at(db_path)?.resolve(git_ref)?; ``` From a naive research on GitHub, there is only one usage[^1] and it seems to be active but still experimental. Pardon my abrupt change breaking your project :( Only when we start considering what we want to expose can Cargo has a clearer boundaries between its subcrates. [^1]: https://github.com/trustification/source-distributed/blob/124ef26081a41330a553483441fca544bedbb473/src/git.rs#L34
It doesn't seem that they need to derive `Serialize`. Is there any other usage I am not aware of?
It was confusing and may lead to inconsistency that people need to pass both `path` and `repo` and ensure they are in sync.
This should be less controversial as the current logic share the same `GitRemote`. You can see the callsite of `GitDatabase::copy_to` use `self.url()`, which then calls into `self.remote.url(). When a `GitDatabase` is created, Cargo also uses `self.remote` as the remote of that `GitDatabase`.
Add message on reusing previous temporary path on failed cargo installs ### What does this PR try to resolve? Fixes rust-lang/cargo#4890 by extending the error message. ### How should we test and review this PR? It was well-tested, a total of 4 tests started failing by changing the error message, which is really good for a first issue, a lot less scary and I got to see the code that tests that the preconditions for this actually works (ie the temp-dir is indeed created).
refactor: git source cleanup
Previously, fetches and clones would routinely fail with a panic that indicated that pack-negotiation can't take longer than 1 round with our previous `Naive` approach. With this version of `gitoxide` there is now faithful support for both the `consecutive` and the `skipping` algorithm and multiple rounds of negotiations, which should make all clones and fetches possible.
Upgrade to `gix` v0.45 for multi-round pack negotiations. Previously, fetches and clones would routinely fail with a panic that indicated that pack-negotiation can't take longer than 1 round with our previous `Naive` approach. With this version of `gitoxide` there is now faithful support for both the `consecutive` and the `skipping` algorithm and multiple rounds of negotiations, which should make all clones and fetches possible. ---- It would be great if we could validate that https://rsproxy.cn is now working - unfortunately I couldn't find the issue to let folks know there.
Nightly doc contains more information and rationale for each item. It's a better fit for those want to extend Cargo's functionality.
doc: point to nightly cargo doc ### What does this PR try to resolve? Nightly doc contains more information and rationale for each item. It's a better fit for those want to extend Cargo's functionality.
docs: doc comments for registry source and index
This is quite straightforward.
We had index format version and index file version. Both sometimes are written as "index version". This is so confusing that I need to check to source code back-and-forth. Today we make "index format version", which refers to the exact representation of a index file, rename to "index schema version". Other index file version of index version should just mean the opaque version of an index file for cache invalidation use.
`RegistryPackage` is a line in a raw registyr index file. This should belong to `index.rs` module.
This piece of code was written before 1.2 and `str::to_lowercase` is an API came out in Rust 1.2.
In certain versions of libcurl when proxy is in use with HTTP/2 multiplexing, connections will continue stacking up. This was fixed in libcurl 8.0.0 in curl/curl@821f6e2 However, Cargo can still link against old system libcurl if it is from a custom built one or on macOS. For those cases, multiplexing needs to be disabled when those versions are detected.
fix: disable multiplexing on macOS for some versions of curl
The output format should be stable I believe, but it turns out not. This is how `git fetch` man page says [1]: ``` <flag> <summary> <from> -> <to> [<reason>] ``` In Git 2.41 they've changed the fetch output a bit [2]. I think let's just loose it to prevent future breakages. [1]: https://git-scm.com/docs/git-fetch#_output [2]: https://github.blog/2023-06-01-highlights-from-git-2-41/
This is a better name to reflect it is from "index" files.
test: loose overly matches for git cli output The output format should be stable I believe, but it turns out not. This is how `git fetch` man page says [^1]: ``` <flag> <summary> <from> -> <to> [<reason>] ``` In Git 2.41 they've changed the fetch output a bit [^2]. I think let's just loose it to prevent future breakages. [^1]: https://git-scm.com/docs/git-fetch#_output [^2]: https://github.blog/2023-06-01-highlights-from-git-2-41/
The goal is that we shouldn't interefere with end-user output when "cargo script"s are used programmatically. The only way to detect this is when piping. CI will also look like this. My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it should have a consistent meaning between local development (`cargo run --manifest-path`) and "script mode" (`cargo`), so I effectively added a new verbosity level in these cases. To get normal output in all cases, add a `-v` like the tests do. Do `-vv` if you want the normal `-v` mode. If you want it always quiet, do `--quiet`. I want to see the default verbosity for interactive "script mode" a bit quieter to the point that all normal output cargo makes is cleared before running the built binary. I am holding off on that now as that could tie into bigger conversations / refactors (see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output).
See #12207 for what all is merged thatt his is trying to cover.
docs(unstable): Update script documentation See #12207 for what all is merged that this is trying to cover.
fix(script): Be quiet on programmatic output ### What does this PR try to resolve? This is a part of #12207 The goal is that we shouldn't interfere with end-user output when "cargo script"s are used programmatically. The only way to detect this is when piping. CI will also look like this. My thought is that if someone does want to do `#!/usr/bin/env -S cargo -v`, it should have a consistent meaning between local development (`cargo run --manifest-path`) and "script mode" (`cargo`), so I effectively added a new verbosity level in these cases. To get normal output in all cases, add a `-v` like the tests do. Do `-vv` if you want the normal `-v` mode. If you want it always quiet, do `--quiet`. I want to see the default verbosity for interactive "script mode" a bit quieter to the point that all normal output cargo makes is cleared before running the built binary. I am holding off on that now as that could tie into bigger conversations / refactors (see https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Re-thinking.20cargo's.20output). ### How should we test and review this PR? Initial writing of tests and refactors are split split out. The tests in the final commit will let you see what impact this had on behavior.
refactor(registry): extract and rearrange items to their own modules
doc: should be `.cargo-ok` It was a mistake. Should be `.cargo-ok`. `.cargo-lock` is a file lock holding for `target/debug` when compiling stuff.
refactor: use macro to remove duplication of workspace inheritable fields getters
Don't try to compile cargo-credential-gnome-secret on non-Linux platforms. This is a followup on #11993 to put the same treatment for `cargo-credential-gnome-secret` so that it builds on non-Linux platforms, substituting the "unsupported" implementation in that case. This will still fail on Linux platforms that don't have libsecret-1-dev installed. I'm not sure how to best treat that, since we do want an error to be generated if the user tries to run `cargo install cargo-credential-gnome-secret`, and they don't have libsecret installed.
Add some more documentation for Source download functions. This adds a little more of a description to the download functions of the `Source` trait, since the behavior wasn't clear to me (and I often forget how these work).
Add READMEs for the credential helpers. This adds some READMEs for these crates. These are pretty bare bones for now, but I suspect they may get extended in the future based on how we decide to deploy them.
I've been dealing with these situations as either the package author, depending on such a package, or supporting users who run into problems that I figure documenting this guidance in a central place means I won't have to repeat myself as often and have to re-find all of the relevant links each time. Alternatives to how this was documented - Use a regular header. All of sections in this document are flat and its hard to see association between them. This also feels like its more on the level of the "note"s. - Put this in a central Recommendations page. I think we should do something more for these when we have more (nothing else in my quick scan stood out as "recommendations" like this). At that point we can have a better idea of how it would work (much like the rule of 3 for generalizing code). I also suspect a "Recommendations" index might be better. - Put this in the FAQ. This can easily be framed as a question and we put the `Cargo.lock` policy in there. I left out talking about alternative proc-macro designs as I feel like treading new ground in community practices is out of the scope of this. See also https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Version.20Requirements.20documentation
docs(ref): Provide guidance on version requirements ### What does this PR try to resolve? I've been dealing with these situations as either the package author, depending on such a package, or supporting users who run into problems that I figure documenting this guidance in a central place means I won't have to repeat myself as often and have to re-find all of the relevant links each time. ### How should we test and review this PR? Alternatives to how this was documented - Use a regular header. All of sections in this document are flat and its hard to see association between them. This also feels like its more on the level of the "note"s. - Put this in a central Recommendations page. I think we should do something more for these when we have more (nothing else in my quick scan stood out as "recommendations" like this). At that point we can have a better idea of how it would work (much like the rule of 3 for generalizing code). I also suspect a "Recommendations" index might be better. - Put this in the FAQ. This can easily be framed as a question and we put the `Cargo.lock` policy in there. I left out talking about alternative proc-macro designs as I feel like treading new ground in community practices is out of the scope of this. ### Additional information See also https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Version.20Requirements.20documentation
Cargo tries to maintain the stability of every output it emits. However, for things like `PackageId` and `SourceId` users should see them as opaque strings. We generally don't change it but we want to reserve the right to change when it is required.
doc: state `PackageId`/`SourceId` string is opaque
Resolved by rust-lang/crates.io#1059 a few weeks ago
update re: multiple versions that differ only in the metadata tag Resolved by rust-lang/crates.io#1059 a few weeks ago
changing the main bin name means modifying a lot of tests i don't see a reason to do that at this point
commit f86b54d Merge: 8505fe0 550ccf0 Author: Travis A. Wagner <[email protected]> Date: Fri Jun 23 13:53:42 2023 -0400 Merge pull request #6 from crablang/update-readme update readme commit 8505fe0 Merge: 5889303 e181e93 Author: Sammi Turner <[email protected]> Date: Fri Jun 23 12:07:50 2023 +0100 Merge pull request #5 from crablang/trav/rename rename commit 550ccf0 Author: Travis Wagner <[email protected]> Date: Fri Jun 23 05:36:15 2023 -0400 update readme commit e181e93 Author: Travis Wagner <[email protected]> Date: Fri Jun 23 05:03:22 2023 -0400 "cargo" -> "crabgo" commit 01ab24a Author: Travis Wagner <[email protected]> Date: Fri Jun 23 04:58:34 2023 -0400 add another bin named "crabgo" with the same path changing the main bin name means modifying a lot of tests i don't see a reason to do that at this point commit 5889303 Merge: c9b89c6 57326f5 Author: Travis A. Wagner <[email protected]> Date: Mon Jun 5 03:13:07 2023 -0400 Merge pull request #3 from crablang/synced synced
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sync with upstream