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

License Header is removed from 3.1.0 release #205

Closed
hazendaz opened this issue May 23, 2022 · 15 comments
Closed

License Header is removed from 3.1.0 release #205

hazendaz opened this issue May 23, 2022 · 15 comments
Assignees
Labels

Comments

@hazendaz
Copy link

I looked over the change log but not seeing any direct mention of what has occurred. My best guess is that something in the jdom 2 upgrade. With 3.0.1, license header in pom.xml is left alone. In 3.1.0, it is deleted. Was this intential? Is there some way to work around the issue?

@Ekryd
Copy link
Owner

Ekryd commented May 23, 2022

Well, the handling of DocType in the XML has been removed due to vulnerability issues. Do you have an example xml?

@hazendaz
Copy link
Author

sure you can run it against this https://github.com/hazendaz/base-parent/blob/master/pom.xml with simple sortpom:sort which Results in license header removal which is just a comment.

image

@dwalluck
Copy link

I see also that <ignore /> has been changed to <ignore/>. I think that as a general rule, sortpom should not modify any kind of whitespace, unless it's also meant to be a code formatting tool.

@hazendaz
Copy link
Author

@dwalluck the 'ignore' can be ignored. I didn't run it with parameters that would not do that. My only issue is the license header is being removed. If I ran all the parameters I use it would have left that bit alone. The example here was just for a quick project to look at that shows the issue.

@hazendaz
Copy link
Author

@dwalluck Use <spaceBeforeCloseEmptyElement>true</spaceBeforeCloseEmptyElement> to ensure the whitespace change doesn't occur. Sorry didn't have that in my example. I have updated my config to include that and re-ran to show the proper screen shot of the consideration on this issue.

image

@dwalluck
Copy link

dwalluck commented May 24, 2022

OK, so your config changed?

I filed #206 and thought that it might be related to that issue.

In my case, I have not changed anything, so I wouldn't expect anything to change (by default, anyway).

@hazendaz
Copy link
Author

hazendaz commented May 24, 2022

I had not changed anything either. The repo I saw this on first is not public. So I added a quick check on one I have that is public that did not use same configuration. If i recall, that configuration with the whitespace removal actually changed with 3.0.0 of this plugin.

So now my private and public repo have same configuration, same issue noted here with license header being removed.

@Ekryd
Copy link
Owner

Ekryd commented May 24, 2022

Thanks for the example. Removing the comment sounds suspicious. I’ll have a look at it.

@Ekryd
Copy link
Owner

Ekryd commented May 24, 2022

This is a problem. When the root element is added to a newly created xml document, all content is cleared (including the comment). I will work on a fix as soon as possible.

@Ekryd Ekryd self-assigned this May 24, 2022
@create-issue-branch
Copy link

@Ekryd
Copy link
Owner

Ekryd commented May 24, 2022

Found the problem. Will issue a new version

@Ekryd Ekryd added the bug label May 24, 2022
@Ekryd
Copy link
Owner

Ekryd commented May 24, 2022

This should be fixed with version 3.1.1. @hazendaz please try it out and report if it fixes your problem

@hazendaz
Copy link
Author

hazendaz commented May 25, 2022 via email

@dwalluck
Copy link

Fixed for me, too.

@Ekryd
Copy link
Owner

Ekryd commented May 25, 2022

Great! I hope that you are satisfied with SortPom and thanks for reporting the issue

@Ekryd Ekryd closed this as completed May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants