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

Suppress the "skipping duplicate package xgboost" warning #1340

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

weikengchen
Copy link
Contributor

If one uses a git path for dependencies of risc0 in Cargo.toml, chances are that the compiler would complain:

warning: skipping duplicate package `xgboost` found at `/Users/cusgadmin/.cargo/git/checkouts/risc0-b4649977e2e81438/341a014/examples/xgboost/methods/guest`

This is due to xgboost's example repo has the name xgboost and its methods/guest also has the name xgboost. Since this is a very boring warning but appears to show up frequently, it makes sense to suppress it, by just naming the xgboost example repo differently.

Copy link

vercel bot commented Jan 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 23, 2024 0:22am

@weikengchen
Copy link
Contributor Author

weikengchen commented Jan 18, 2024

could not reproduce the following CI error locally.

Run python license-check.py
examples/groth16-verifier/groth16/tests/groth16.rs: invalid header!
expected: // Copyright 2024 RISC Zero, Inc.
actual: // Copyright 2023 RISC Zero, Inc.
examples/groth16-verifier/methods/build.rs: invalid header!
expected: // Copyright 2024 RISC Zero, Inc.
actual: // Copyright 2023 RISC Zero, Inc.
examples/groth16-verifier/methods/guest/src/bin/groth16_verifier.rs: invalid header!
expected: // Copyright 2024 RISC Zero, Inc.
actual: // Copyright 2023 RISC Zero, Inc.
examples/groth16-verifier/methods/src/lib.rs: invalid header!
expected: // Copyright 2024 RISC Zero, Inc.
actual: // Copyright 2023 RISC Zero, Inc.
examples/groth16-verifier/src/main.rs: invalid header!
expected: // Copyright 2024 RISC Zero, Inc.
actual: // Copyright 2023 RISC Zero, Inc.
Error: Process completed with exit code 1.

@flaub
Copy link
Member

flaub commented Jan 18, 2024

Could you update the copyright header's year on each of the effected files?

@weikengchen
Copy link
Contributor Author

Let me dig out what happens... It is weird. The problem is that the header is indeed correct on the source repo:
https://github.com/l2iterative/risc0/blob/remove-xgboost-warning/examples/groth16-verifier/groth16/tests/groth16.rs#L1

// Copyright 2024 RISC Zero, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//

@weikengchen
Copy link
Contributor Author

weikengchen commented Jan 18, 2024

Ok. @flaub I think it should work now. I realized that my previous commit (just added after the CI failure) here that merges the upstream should have fixed the issue.
@weikengchen
Merge branch 'main' into remove-xgboost-warning
44709c0

It is related to including the fix of headers in #1317 (comment).

@flaub flaub enabled auto-merge (rebase) January 18, 2024 03:17
@flaub flaub disabled auto-merge January 18, 2024 03:17
@flaub flaub enabled auto-merge (squash) January 18, 2024 03:17
auto-merge was automatically disabled January 18, 2024 03:58

Head branch was pushed to by a user without write access

@weikengchen
Copy link
Contributor Author

It appears that Cargo.lock of examples needs to be updated because a new entry would be added:

[[package]]
name = "xgboost-example"
version = "0.1.0"
dependencies = [
 "forust-ml",
 "risc0-zkvm",
 "rmp-serde",
 "serde",
 "serde_json",
 "xgboost-methods",
]

A commit is pushed trying to fix the remaining CI errors.

@weikengchen
Copy link
Contributor Author

weikengchen commented Jan 18, 2024

An automatic run of cargo update will create a lot of changes to Cargo.lock... if it is not desirable, please let me know so I can try to minimize the change to Cargo.lock to be name-only.

After you review the changes, a new approval would be needed to rerun the CI.

@flaub
Copy link
Member

flaub commented Jan 18, 2024

The only thing that's needed is to run cargo check from the examples directory. This will update the Cargo.lock file to match what's in the Cargo.toml. This should cause Cargo.lock to be changed, so just commit that to this PR.

@weikengchen
Copy link
Contributor Author

The requested change has been made. cargo check instead of cargo update fixes the issue. Now there is only the following change in this PR.

[[package]]
- name = "xgboost"
+ name = "xgboost-example"

@weikengchen
Copy link
Contributor Author

weikengchen commented Jan 18, 2024

Just an update that a new error has appeared.

warning: skipping duplicate package `methods` found at `/Users/cusgadmin/.cargo/git/checkouts/risc0-b4649977e2e81438/39a6fd8/examples/groth16-verifier/methods`

let me take a look as well since we clearly do not want to spend one more PR for this...

@weikengchen
Copy link
Contributor Author

A new commit has fixed the same issue about groth16-verify as well. Ready for review.

@SchmErik
Copy link
Contributor

@weikengchen we can't merge because this is outdated. Could you update your branch so we can merge?

@weikengchen
Copy link
Contributor Author

Updated. Realized that one problem with making a PR from organization-owned repo is that I couldn't auto-grant permission to modify to the RISC Zero team...

@flaub flaub enabled auto-merge (squash) January 23, 2024 00:39
@flaub flaub merged commit 6c60dcf into risc0:main Jan 23, 2024
23 checks passed
@flaub flaub added this to the 0.20.0 milestone Jan 23, 2024
flaub pushed a commit that referenced this pull request Jan 24, 2024
If one uses a git path for dependencies of risc0 in Cargo.toml, chances
are that the compiler would complain:

```
warning: skipping duplicate package `xgboost` found at `/Users/cusgadmin/.cargo/git/checkouts/risc0-b4649977e2e81438/341a014/examples/xgboost/methods/guest`
```

This is due to xgboost's example repo has the name `xgboost` and its
methods/guest also has the name `xgboost`. Since this is a very boring
warning but appears to show up frequently, it makes sense to suppress
it, by just naming the xgboost example repo differently.
flaub pushed a commit that referenced this pull request Jan 24, 2024
If one uses a git path for dependencies of risc0 in Cargo.toml, chances
are that the compiler would complain:

```
warning: skipping duplicate package `xgboost` found at `/Users/cusgadmin/.cargo/git/checkouts/risc0-b4649977e2e81438/341a014/examples/xgboost/methods/guest`
```

This is due to xgboost's example repo has the name `xgboost` and its
methods/guest also has the name `xgboost`. Since this is a very boring
warning but appears to show up frequently, it makes sense to suppress
it, by just naming the xgboost example repo differently.
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.

None yet

3 participants