Skip to content

Comments

PARQUET-2155: Upgrade protobuf version to 3.17.3#973

Merged
ggershinsky merged 3 commits intoapache:masterfrom
sunchao:upgrade-protoc
Jul 19, 2022
Merged

PARQUET-2155: Upgrade protobuf version to 3.17.3#973
ggershinsky merged 3 commits intoapache:masterfrom
sunchao:upgrade-protoc

Conversation

@sunchao
Copy link
Member

@sunchao sunchao commented Jun 10, 2022

Currently the repo can't be compiled with M1 Mac:

com.google.protobuf:protoc:exe:osx-aarch_64:3.16.1 was not found in https://jitpack.io/ during a previous attempt.

since the artifact for M1 arch was not published: https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.16.1/

This upgrade the protobuf version to 3.17.3 to support compiling on M1 Mac:

https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.17.3/ (with osx_aarch_64 artifact)

Make sure you have checked all steps below.

Jira

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

  • My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

@sunchao sunchao changed the title Upgrade protobuf version to 3.20.1 PARQUET-2155: Upgrade protobuf version to 3.20.1 Jun 10, 2022
@steveloughran
Copy link
Contributor

this is a different solution to mine, which just changed the protoc artifact. i think my one is lower risk, but this one can address other protobuf issues

@steveloughran
Copy link
Contributor

...but the #970 patch should add a comment saying "not needed once protobuf is incremented to 3.20+"

@sunchao
Copy link
Member Author

sunchao commented Jun 10, 2022

To reduce the risk, we can also change 3.20.1 to 3.17.1 which seems to be the lowest version that has the aarch_64 artifact.

@sunchao
Copy link
Member Author

sunchao commented Jun 10, 2022

cc @ggershinsky @shangxinli

@shangxinli
Copy link
Contributor

Yeah, we can do 3.20.1 later.

@sunchao sunchao changed the title PARQUET-2155: Upgrade protobuf version to 3.20.1 PARQUET-2155: Upgrade protobuf version to 3.17.1 Jul 2, 2022
@sunchao sunchao changed the title PARQUET-2155: Upgrade protobuf version to 3.17.1 PARQUET-2155: Upgrade protobuf version to 3.17.3 Jul 2, 2022
@sunchao
Copy link
Member Author

sunchao commented Jul 2, 2022

updated

@sunchao
Copy link
Member Author

sunchao commented Jul 13, 2022

gently ping @shangxinli @ggershinsky

@ggershinsky
Copy link
Contributor

sure. if no other input by the end of this week, I'll merge it then.

@shangxinli
Copy link
Contributor

LGTM

@ggershinsky ggershinsky merged commit 019361e into apache:master Jul 19, 2022
@steveloughran
Copy link
Contributor

now this is merged in, should the jira be closed?

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.

4 participants