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

Bump 'prost' version to 0.11 #152

Closed
wants to merge 2 commits into from

Conversation

fabricedesre
Copy link
Contributor

This helps downstream crates to not have duplicate version of 'hashbrown'

@fabricedesre
Copy link
Contributor Author

Ping @vmx :)

Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

I usually do version bumps during the release, so reverting that commit would be nice.

We didn't upgrade to prost 0.11 as it doesn't bundle protoc anymore. I think there's some alternative that does that. So either leave it at 0.10, or dig into the alternative :)

@fabricedesre
Copy link
Contributor Author

We didn't upgrade to prost 0.11 as it doesn't bundle protoc anymore. I think there's some alternative that does that. So either leave it at 0.10, or dig into the alternative :)

I used the recommended protobuf-src crate to vendor protoc. If that's fine, can you publish a new version?

This now uses protobuf-src to vendor protoc
@fabricedesre fabricedesre changed the title Bump 'cached' version to 0.38 Bump 'prost' version to 0.11 Sep 24, 2022
@vmx
Copy link
Member

vmx commented Sep 26, 2022

As you mention downstream crates. Please be aware that this implementation of DAG-PB is currently not spec compliant. The tracking issue is https://bugzilla.mozilla.org/show_bug.cgi?id=1792370. #139.

@fabricedesre
Copy link
Contributor Author

As you mention downstream crates. Please be aware that this implementation of DAG-PB is currently not spec compliant. The tracking issue is https://bugzilla.mozilla.org/show_bug.cgi?id=1792370.

sorry, how is that bug relevant here?

@vmx
Copy link
Member

vmx commented Sep 26, 2022

As you mention downstream crates. Please be aware that this implementation of DAG-PB is currently not spec compliant. The tracking issue is https://bugzilla.mozilla.org/show_bug.cgi?id=1792370.

sorry, how is that bug relevant here?

Sorry, copy and paste failure, I've meant #139.

@fabricedesre
Copy link
Contributor Author

I still don't understand - this PR is not changing any behavior, just trying to reduce dependencies duplicates.

@vmx
Copy link
Member

vmx commented Sep 27, 2022

I still don't understand - this PR is not changing any behavior, just trying to reduce dependencies duplicates.

My comment was related directly to this PR. It was just a note, that users that use DAG-PB should be aware that the current implementation isn't spec compliant. As you are interested in reducing duplicated dependencies, I thought it would be appropriate to mention that.

@vmx
Copy link
Member

vmx commented Sep 27, 2022

Does anyone know how to fix the Windows CI, so that it can compile protoc?

@fabricedesre
Copy link
Contributor Author

Does anyone know how to fix the Windows CI, so that it can compile protoc?

Looks like it's blocked on protobuf-src itself: MaterializeInc/rust-protobuf-native#4 😢

@vmx
Copy link
Member

vmx commented Sep 28, 2022

As protobuf-src is using autocxx, https://github.com/google/autocxx/blob/b245636274be4206be5849cf3355b315a609969c/.github/workflows/ci.yml might give some ideas on how to get it compiled on Windows.

@fabricedesre
Copy link
Contributor Author

Closing in favor of #161

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.

2 participants