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

[BLOCKED] upgrade gix to v0.41 to fix flaky auth tests. #11831

Closed
wants to merge 1 commit into from

Conversation

Byron
Copy link
Member

@Byron Byron commented Mar 10, 2023

Follow-up of: #11830 which provides the first commit c890c64.

The latest release (https://github.com/Byron/gitoxide/releases/tag/gix-v0.41.0) makes writing to the credential helper non-fallible which is useful if the helper program doesn't read from stdin and is in line with of git implements it as well.

Blocked

This fix also undoes a pin tempfile to version 3.3 which at this time causes build issues for some platforms if tempfile version 3.4 or newer would be used.

Thus this PR cannot be merged until tempfile will not cause build failures.
This also means that cargo can't upgrade to any newer version of gitoxide until tempfile isn't an issue anymore, also blocking the shallow clone feature for example.

@weihanglo, maybe you can replace this line with references to issues to wait for.

Fixes #11821

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 10, 2023
@Byron Byron force-pushed the fix-11821 branch 2 times, most recently from 99c98e6 to 41412a1 Compare March 11, 2023 12:05
bors added a commit that referenced this pull request Mar 11, 2023
Disable flaky auth tests when `gitoxide` runs them

The proper fix is in https://github.com/Byron/gitoxide/releases/tag/gix-v0.41.0
which unfortunately can't be used as it also comes with the latest `tempfile` v3.4
which causes other issues when compiling on some platforms.

Thus we first disable the flaky tests, and re-enable them with the `gix` upgrade
which should be possible once `tempfile` doesn't hinder `cargo` on some platforms
anymore.

Related to #11821

This PR is supposed to be followed up by #11831 which re-enables the flaky tests and fixes them properly by upgrading `gix` which contains the fix.
@weihanglo weihanglo mentioned this pull request Mar 14, 2023
7 tasks
@bors
Copy link
Contributor

bors commented Apr 3, 2023

☔ The latest upstream changes (presumably #11928) made this pull request unmergeable. Please resolve the merge conflicts.

upgrade `gix` to v0.41 to fix flaky auth tests and revert commit c890c64.

The latest release (https://github.com/Byron/gitoxide/releases/tag/gix-v0.41.0)
makes writing to the credential helper non-fallible which is useful if the helper
program doesn't read from stdin and is in line with of `git` implements it as well.

This fix also undoes a pin `tempfile` to version 3.3 which at this time
causes build issues for some platforms if `tempfile` version 3.4 or newer would be used.

Thus this PR cannot be merged until `tempfile` will not cause build failures.
This also means that `cargo` can't upgrade to any newer version of `gitoxide` until `tempfile` isn't an
issue anymore, also blocking the shallow clone feature for example.

@weihanglo, maybe you can replace this line with references to issues to wait for.
bors added a commit that referenced this pull request Apr 15, 2023
Make cargo a workspace

### What does this PR try to resolve?

The first step of making cargo a workspace.

Benefits:

* Dogfooding ourselves.
* Unblock #11831: It got stuck because the new version of tempfile using `windows-sys` but some issues haven't yet be solved in rust-lang/rust.
* Make `cargo xtask` or similar developer workflow possible (e.g., #11717)
* Having our own Cargo.lock, so our CI can cover the exact binary going to ship. Also free Cargo from CI breaks due to dependency patch releases.
* Probably more? Please add them by yourself.

### How should we test and review this PR?

Please review it commit by commit. A companion PR is here rust-lang/rust#109133, and should be reviewed together.

### Unresolved issues

To limit the scope of this pull request, the following issues are intentionally left unresolved. They will be addressed right after this pull request gets merged.

- [x] Make `benches/capture` and `benches/capture` workspace members. (Addressed with 2cf9718)
- [x] Make `crates/resolver-tests` a workspace member. (Addressed with #11886)
- [ ] ~~Fix clippy warnings and re-enable clippy check in CI for all workspace members.~~
  - Blocked on rust-lang/rfcs#3389 so we can more easily propagate our clippy settings
- [ ] Fix rustdoc warnings and re-enable rustdoc check in CI for all workspace members.
- [ ] Fix `linkchecker.sh` warnings in CI (#11851 (comment))
- [ ] Leverage workspace flag `--workspace` when running `cargo build` or `cargo test`, instead of using flag `-p`.
- [ ] Leverage glob syntax when probing members in `[workspace]` in Cargo.toml (i.e., `crates/*`).

### Additional information

This depends on prior works from `@Muscraft` and `@ehuss.` Credits to them!
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Byron, I believe this is mergeable if you update the lockfile!

@weihanglo
Copy link
Member

Or maybe this isn't needed anymore if we can make shallow clones PR merged in a timely manner?

@Byron
Copy link
Member Author

Byron commented Apr 18, 2023

Or maybe this isn't needed anymore if we can make shallow clones PR merged in a timely manner?

Yes, I think that's preferable as merging this early doesn't seem to have any benefit. The shallow-clones PR includes this commit as well.

For that reason, closing.

@Byron Byron closed this Apr 18, 2023
@Byron Byron deleted the fix-11821 branch April 18, 2023 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gitoxide auth tests sporadically failing
4 participants