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

[FLINK-24964] Upgrade frontend-maven-plugin #17795

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Conversation

maver1ck
Copy link
Contributor

What is the purpose of the change

Upgrade frontend-maven-plugin to fix compilation issue on Apple M1

Brief change log

  • change plugin version in pom.xml

Verifying this change

Verified by compilation.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): yes
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no

@flinkbot
Copy link
Collaborator

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit dd59ec6 (Mon Nov 15 12:09:03 UTC 2021)

Warnings:

  • 1 pom.xml files were touched: Check for build and licensing issues.
  • No documentation files were touched! Remember to keep the Flink docs up to date!
  • This pull request references an unassigned Jira ticket. According to the code contribution guide, tickets need to be assigned before starting with the implementation work.

Mention the bot in a comment to re-run the automated checks.

Review Progress

  • ❓ 1. The [description] looks good.
  • ❓ 2. There is [consensus] that the contribution should go into to Flink.
  • ❓ 3. Needs [attention] from.
  • ❓ 4. The change fits into the overall [architecture].
  • ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.


The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commands
The @flinkbot bot supports the following commands:

  • @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
  • @flinkbot approve all to approve all aspects
  • @flinkbot approve-until architecture to approve everything until architecture
  • @flinkbot attention @username1 [@username2 ..] to require somebody's attention
  • @flinkbot disapprove architecture to remove an approval you gave earlier

@flinkbot
Copy link
Collaborator

flinkbot commented Nov 15, 2021

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@zentol
Copy link
Contributor

zentol commented Nov 15, 2021

We cannot do this upgrade because this version requires Maven 3.6.0, while we are still using 3.2.5.

@maver1ck
Copy link
Contributor Author

@zentol I will try to check if there is a plugin version fixing M1 bug and working on maven 3.2.5

@maver1ck
Copy link
Contributor Author

@zentol I think I found proper version :)

@rmetzger
Copy link
Contributor

While testing your change, the build failed with:

[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 07:35 min
[INFO] Finished at: 2021-11-16T20:42:42+01:00
[INFO] Final Memory: 581M/6384M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.xolstice.maven.plugins:protobuf-maven-plugin:0.5.1:test-compile (default) on project flink-parquet_2.12: Missing:
[ERROR] ----------
[ERROR] 1) com.google.protobuf:protoc:exe:osx-aarch_64:3.5.1
[ERROR]
[ERROR] Try downloading the file manually from the project website.
[ERROR]
[ERROR] Then, install it using the command:
[ERROR] mvn install:install-file -DgroupId=com.google.protobuf -DartifactId=protoc -Dversion=3.5.1 -Dclassifier=osx-aarch_64 -Dpackaging=exe -Dfile=/path/to/file
[ERROR]
[ERROR] Alternatively, if you host your own repository you can deploy the file there:
[ERROR] mvn deploy:deploy-file -DgroupId=com.google.protobuf -DartifactId=protoc -Dversion=3.5.1 -Dclassifier=osx-aarch_64 -Dpackaging=exe -Dfile=/path/to/file -Durl=[url] -DrepositoryId=[id]
[ERROR]
[ERROR] Path to dependency:
[ERROR] 1) org.apache.flink:flink-parquet_2.12:jar:1.15-SNAPSHOT
[ERROR] 2) com.google.protobuf:protoc:exe:osx-aarch_64:3.5.1
[ERROR]
[ERROR] ----------
[ERROR] 1 required artifact is missing.
[ERROR]
[ERROR] for artifact:
[ERROR] org.apache.flink:flink-parquet_2.12:jar:1.15-SNAPSHOT

Ideally we shouldn't require people to perform manual steps to get a clean build. Did it work out of the box for you?

@maver1ck
Copy link
Contributor Author

maver1ck commented Nov 16, 2021

No. I also stopped on this step. But I think we should fix M1 problems one by one :)

Also: os72/protoc-jar#93

@rmetzger
Copy link
Contributor

Thanks for confirming that you are also seeing this issue.

Sadly, the frontend-maven-plugin loads the x86 version of node into flink-runtime-web/web-dashboard/node, requiring rosetta to run parts of the Flink build.
I assume this is fine for now, given how well rosetta works, but its not a permanent solution (Rosetta was around for ~5 years during the transition from PowerPC to Intel in 2006)

@maver1ck
Copy link
Contributor Author

I think this is good workaround for now.
Upgrading node.js to version with aarch support for sure will be more demanding.

@maver1ck
Copy link
Contributor Author

@rmetzger
I think the best solution for protoc will be adding this file to maven central. Do we have any friends in protobuf project ?
https://developers.google.com/protocol-buffers/

@maver1ck maver1ck changed the title [FLINK-23230] Upgrade frontend-maven-plugin [FLINK-24964] Upgrade frontend-maven-plugin Nov 19, 2021
@MartijnVisser
Copy link
Contributor

@rmetzger Do you think we should merge this and create a follow up ticket?

@rmetzger
Copy link
Contributor

Sorry for the delay. I'm currently on parental leave and have very little time in front of my screens ;)

Do we have any friends in protobuf project ?

Not that I'm aware of :)

Yes, let's merge this and do a separate ticket for the remaining issue(s).

@rmetzger
Copy link
Contributor

I'll merge it myself asap (maybe tonight or tomorrow), but I'd also be happy if another Flink committer would merge it in the meantime.

@MartijnVisser
Copy link
Contributor

@AHeise Please merge :)

@zentol zentol merged commit e431e6b into apache:master Nov 22, 2021
@maver1ck maver1ck deleted the FLINK-23230 branch November 23, 2021 10:08
@rmetzger
Copy link
Contributor

Thanks @zentol !

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.

5 participants