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

verify-release-candidate.sh includes unpassable cargo publish checks #881

Closed
alamb opened this issue Oct 29, 2021 · 3 comments · Fixed by #882
Closed

verify-release-candidate.sh includes unpassable cargo publish checks #881

alamb opened this issue Oct 29, 2021 · 3 comments · Fixed by #882
Assignees
Labels

Comments

@alamb
Copy link
Contributor

alamb commented Oct 29, 2021

Describe the bug
The newly added cargo publish checks I added in #856 (see https://github.com/apache/arrow-rs/pull/856/files#diff-82f32771b6ac9daaadbfb77bd77062d63e131ad18ecd9014ff90eff17865964fR137-R152) do not actually work for any crate other than arrow

The issue is that cargo publish ignores the 'path' annotations in Cargo.toml

Basically the issue is that they can't find the correct version of arrow for some reason

Here is an example failure:


/var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT/apache-arrow-rs-6.1.0 /var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT
+ pushd arrow-flight
/var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT/apache-arrow-rs-6.1.0/arrow-flight /var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT/apache-arrow-rs-6.1.0 /var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT
+ cargo publish --dry-run
    Updating crates.io index
   Packaging arrow-flight v6.1.0 (/private/var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT/apache-arrow-rs-6.1.0/arrow-flight)
error: failed to prepare local package for uploading

Caused by:
  failed to select a version for the requirement `arrow = "^6.1.0"`
  candidate versions found which didn't match: 6.0.0, 5.5.0, 5.4.0, ...
  location searched: crates.io index
  required by package `arrow-flight v6.1.0 (/private/var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT/apache-arrow-rs-6.1.0/arrow-flight)`
+ cleanup
+ '[' no = yes ']'
+ echo 'Failed to verify release candidate. See /var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT for details.'
Failed to verify release candidate. See /var/folders/s3/h5hgj43j0bv83shtmz_t_w400000gn/T/arrow-6.1.0.XXXXX.1Si6YRcT for details.

To Reproduce

./dev/release/verify-release-candidate.sh 6.1.0 1

Expected behavior
Check should pass

Additional context
#856

@houqp
Copy link
Member

houqp commented Oct 30, 2021

@alamb in order for path dependency to work, we need to supply an extra version that matches with the version we have in the arrow crate. For example, this is how we made it work for datafusion: https://github.com/apache/arrow-datafusion/blob/master/ballista/rust/core/Cargo.toml#L48

@alamb
Copy link
Contributor Author

alamb commented Nov 2, 2021

Thanks @houqp -- I messed around a little with this

The Cargo.toml in arrow-flight already uses https://github.com/apache/arrow-rs/blob/active_release/arrow-flight/Cargo.toml#L29

[dependencies]
arrow = { path = "../arrow", version = "6.1.0" }

In fact cargo build works just fine -- the the issue is that cargo publish --dry-run seems to ignore the local path override.

This behavior kind of makes sense given the discussion on https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies

Note: crates.io does not allow packages to be published with path dependencies (path dev-dependencies are ignored). See the Multiple locations section for a fallback alternative.

So I don't have any other ideas at this time 😢

@houqp
Copy link
Member

houqp commented Nov 5, 2021

ha, you are right @alamb , the upstream path based dependencies need to be published to crates first.

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

Successfully merging a pull request may close this issue.

2 participants