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

[GEOT-7442] Make ErrorProne run on Windows #4479

Merged

Conversation

bestrauss
Copy link
Contributor

@bestrauss bestrauss commented Sep 16, 2023

GEOT-7442 Powered by Pull Request Badge

Changes to file pom.xml to make ErrorProne run on Windows, where file names, including ${project.build.directory}, contain backslashes.

Checklist

For core and extension modules:

  • [N/A] New unit tests have been added covering the changes.
  • [N/A] Documentation has been updated (if change is visible to end users).
  • [N/A] There is an issue in GeoTools Jira (except for changes not visible to end users).
  • [N/A] Commit message(s) must be in the form [GEOT-XYZW] Title of the Jira ticket.
  • [N/A] Bug fixes and small new features are presented as a single commit.
  • [N/A] The commit targets a single objective (if multiple focuses cannot be avoided, each one is in its own commit, and has a separate ticket describing it).

@bradh
Copy link
Contributor

bradh commented Sep 16, 2023

Have restarted the failed CI check - looked unrelated.

@bestrauss
Copy link
Contributor Author

@bradh Thanks!

@bestrauss
Copy link
Contributor Author

@aaime Here's the ErrorProne/Windows PR, ready.

Copy link
Member

@mprins mprins left a comment

Choose a reason for hiding this comment

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

Using build-helper-maven-plugin is almost always a hack either to work around some setup error breaking maven conventions or buggy behaviour in a jdk or plugin. Doing this for all build environments seems wrong, if at all it should only change the value on broken windows shells

@bestrauss
Copy link
Contributor Author

bestrauss commented Sep 17, 2023

@mprins

I agree that preferably, the problem should be solved in ErrorProne. Developers over there have been aware of the issue since June 2021: google/error-prone#2394

In the meantime, in geoserver a workaround has been merged in March 2023: geoserver/geoserver#6605

The given solution changes the value only if it contains backslashes. Unix style paths or other paths which do not contain hazardous backslashes remain unchanged.

@bestrauss bestrauss force-pushed the GEOT-7442-Make-ErrorProne-run-on-Windows branch from 610fc99 to bbf360b Compare September 17, 2023 13:28
@bestrauss
Copy link
Contributor Author

I have simplified the changes of file pom.xml. Only one line is changed.

${project.build.directory}/generated-sources/.*

is changed to

\Q${project.build.directory}\E/generated-sources/.*

This solution was used in geoserver, too.

@aaime aaime merged commit 6e9ef6f into geotools:main Sep 18, 2023
@aaime
Copy link
Member

aaime commented Sep 18, 2023

Merged, thanks!

@aaime
Copy link
Member

aaime commented Nov 17, 2023

The backport to 29.x failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply bbf360bbf6... [GEOT-7442] Make ErrorProne run on Windows
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

stdout
Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-29.x 29.x
# Navigate to the new working tree
cd .worktrees/backport-29.x
# Create a new branch
git switch --create backport-4479-to-29.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick bbf360bbf65de1a29597101835bd504b4428d132
# Push it to GitHub
git push --set-upstream origin backport-4479-to-29.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-29.x

Then, create a pull request where the base branch is 29.x and the compare/head branch is backport-4479-to-29.x.

@aaime
Copy link
Member

aaime commented Nov 17, 2023

The backport to 28.x failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply bbf360bbf6... [GEOT-7442] Make ErrorProne run on Windows
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

stdout
Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-28.x 28.x
# Navigate to the new working tree
cd .worktrees/backport-28.x
# Create a new branch
git switch --create backport-4479-to-28.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick bbf360bbf65de1a29597101835bd504b4428d132
# Push it to GitHub
git push --set-upstream origin backport-4479-to-28.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-28.x

Then, create a pull request where the base branch is 28.x and the compare/head branch is backport-4479-to-28.x.

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.

4 participants