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

ci: Test minimum versions of dependencies #2112

Merged
merged 7 commits into from
Oct 29, 2023
Merged

Conversation

max-sixty
Copy link
Member

This will fail because of duckdb/duckdb-rs#134, so we'll wait for that to resolve before merging

@max-sixty max-sixty changed the title ci: Add a job to test ci: Test minimum versions of dependencies Mar 12, 2023
@eitsupi
Copy link
Member

eitsupi commented May 15, 2023

Currently this seems to fail because it cannot successfully download tiberius's dependency rust-encoding (which abandoned years ago...).

error: process didn't exit successfully: `/home/runner/.rustup/toolchains/1.65.0-x86_64-unknown-linux-gnu/bin/cargo metadata --format-version=1` (exit status: 101)
--- stderr
 Downloading crates ...
error: failed to download `encoding-index-tradchinese v1.0.20140915`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/encoding-index-tradchinese-1.0.20140915/Cargo.toml`

Caused by:
  library target names cannot contain hyphens: encoding-index-tradchinese

error: process didn't exit successfully: `/home/runner/.rustup/toolchains/1.65.0-x86_64-unknown-linux-gnu/bin/cargo hack test` (exit status: 1)
Error: Process completed with exit code 1.

This is obviously a development dependency and would have a chance of passing the test if it could be excluded from the test, but is there a way to exclude it?

@max-sixty
Copy link
Member Author

Ah, interesting @eitsupi. I think the best way is to figure out where it's coming from — it's not that easy but I've used cargo tree -i successfully to figure out the original issue. Then we can raise upstream / remove a dependency from prql...

@eitsupi
Copy link
Member

eitsupi commented May 16, 2023

@max-sixty tiberius is this.

tiberius = {version = "0.12", features = ["sql-browser-tokio", "bigdecimal", "time"]}

This is a test dependency, so it is not worth doing a min-version check here. But I don't know how to exclude this.

(BTW, I completely forgot to mention that this crate issue was pointed out earlier on this issue #2351)

@max-sixty
Copy link
Member Author

I see you put in prisma/tiberius#292! Thanks.

If we can exclude it, great. Otherwise I guess we wait for tiberius to move to a new crate...

@max-sixty
Copy link
Member Author

The docs don't say anything about exclusions, so I think we wait for prisma/tiberius#292

@max-sixty
Copy link
Member Author

I think we need to wait for the next release of clio, to upgrade minimal clap to clap 4 (CC @aj-bagwell , without wanting to disturb!)

@aj-bagwell
Copy link
Contributor

clio will work with clap 3 and clap 4, you should be fine to bump you minumum clap version. Cargo sometimes gets confused and adds in two versions of clap and uses one for clio and a different one for prql which means you have to fix the lock file by hand.

@max-sixty
Copy link
Member Author

clio will work with clap 3 and clap 4, you should be fine to bump you minumum clap version. Cargo sometimes gets confused and adds in two versions of clap and uses one for clio and a different one for prql which means you have to fix the lock file by hand.

Yes, I'm not sure how the resolver does this, but running the min version tests downgrades clap to 3, and then the code breaks.

But sounds like this is a cargo problem (or at least a PRQL problem) rather than a Clio problem.

@max-sixty
Copy link
Member Author

FYI this seems to come from when clap refers to a clap_lex struct RawArgs:

   Compiling clap v3.2.0
error[E0599]: no method named `is_end` found for mutable reference `&mut RawArgs` in the current scope
   --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-3.2.0/src/parser/parser.rs:434:63
    |
434 |                 if cfg!(feature = "unstable-v4") || !raw_args.is_end(&args_cursor) {
    |                                                               ^^^^^^ method not found in `&mut RawArgs`

For more information about this error, try `rustc --explain E0599`.

I guess we may need to wait until clap 3 is unsupported by any dependency

This will fail because of duckdb/duckdb-rs#134, so we'll wait for that to resolve before merging
@max-sixty
Copy link
Member Author

This works now! Merging...

@max-sixty max-sixty merged commit f6915b6 into PRQL:main Oct 29, 2023
71 checks passed
@max-sixty max-sixty deleted the min-versions branch October 29, 2023 22:18
@eitsupi
Copy link
Member

eitsupi commented Oct 29, 2023

Awesome! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants