Skip to content
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

Merged
merged 197 commits into from
Jul 9, 2023
Merged

update #8

merged 197 commits into from
Jul 9, 2023

Conversation

trvswgnr
Copy link
Contributor

@trvswgnr trvswgnr commented Jul 9, 2023

sync with upstream

weihanglo and others added 30 commits May 31, 2023 22:28
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).
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
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/
epage and others added 29 commits June 22, 2023 19:27
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
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
@trvswgnr trvswgnr merged commit f1a90e0 into master Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.