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

Fix build inside git worktrees by working around git-commit-id #639 #18046

Merged

Conversation

xkrogen
Copy link
Member

@xkrogen xkrogen commented Jun 26, 2023

Description

Explicitly configure the dotGitDirectory used by the git-commit-id-maven-plugin to be the root of the (multi-module) project using the air.main.basedir property.

This is needed when using git worktrees, as the plugin incorrectly detects the wrong directory in which to run git commands. If git-commit-id/git-commit-id-maven-plugin#639 or git-commit-id/git-commit-id-maven-plugin#215 are resolved, this will become unnecessary. When Trino upgrades to Maven 4.0.0, assuming the workaround is still needed, we can update to using the new project.rootDirectory property (see MNG-7038) instead of the Airbase one.

Additional context and related issues

This fixes #18027; see the discussion there for more details.

Release notes

(X) This is not user-visible or docs only and no release notes are required.

@cla-bot
Copy link

cla-bot bot commented Jun 26, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

1 similar comment
@cla-bot
Copy link

cla-bot bot commented Jun 26, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@xkrogen xkrogen force-pushed the xkrogen-fix-commit-id-plugin-in-worktree branch from 8eb9a27 to ed8fadc Compare June 27, 2023 16:26
@cla-bot
Copy link

cla-bot bot commented Jun 27, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@xkrogen
Copy link
Member Author

xkrogen commented Jun 27, 2023

Looks like the trino-hive test failed on something flaky; it succeeded in the previous run (which was identical except for being split into two commits rather than being squashed):

Error:  Tests run: 3855, Failures: 1, Errors: 0, Skipped: 154, Time elapsed: 1,894.981 s <<< FAILURE! - in TestSuite
Error:  io.trino.plugin.hive.TestHiveConnectorTest.testWriterTasksCountLimitPartitionedScaleWritersEnabled  Time elapsed: 11.929 s  <<< FAILURE!
java.lang.AssertionError: expected [2] but found [4]

I sent in my CLA a couple of days ago but haven't heard back yet.

pom.xml Outdated Show resolved Hide resolved
@cla-bot
Copy link

cla-bot bot commented Jun 28, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@cla-bot cla-bot bot added the cla-signed label Jul 10, 2023
@xkrogen xkrogen force-pushed the xkrogen-fix-commit-id-plugin-in-worktree branch from d75541a to c7a05ce Compare July 10, 2023 23:47
@wendigo
Copy link
Contributor

wendigo commented Jul 11, 2023

Please squash commits into single one @xkrogen

@xkrogen xkrogen force-pushed the xkrogen-fix-commit-id-plugin-in-worktree branch from 173f72a to 702c995 Compare July 11, 2023 17:12
@xkrogen
Copy link
Member Author

xkrogen commented Jul 11, 2023

Done! Thanks @wendigo

@wendigo
Copy link
Contributor

wendigo commented Jul 11, 2023

@xkrogen I'll merge this change when tests are completed

@wendigo wendigo merged commit 80afde9 into trinodb:master Jul 12, 2023
@github-actions github-actions bot added this to the 422 milestone Jul 12, 2023
@wendigo
Copy link
Contributor

wendigo commented Jul 12, 2023

@xkrogen thanks for the fix and congratulations on your first contribution to Trino 🎉

@xkrogen xkrogen deleted the xkrogen-fix-commit-id-plugin-in-worktree branch July 12, 2023 14:58
@xkrogen
Copy link
Member Author

xkrogen commented Jul 12, 2023

Woohoo! :)

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

Successfully merging this pull request may close these issues.

Trino can't build from a git worktree
3 participants