You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/05/20 18:14:07 UTC
[GitHub] [maven-shade-plugin] cstamas opened a new pull request #93: Improve and simplify ServicesResourceTransformer
cstamas opened a new pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93
Payback time: As ServicesResourceTransformer did "inspire"
SisuIndexResourceTransformer, and the latter with elharo
help came out rather nice, I think it is "payback
time" and improve similarly the original inspiration as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] hboutemy edited a comment on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
hboutemy edited a comment on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-846365632
platform dependent newline is not an issue, it's normal, even if sometimes it adds some complexity: it does not block Reproducible Builds, see https://github.com/jvm-repo-rebuild/reproducible-central where I reproduced many releases done on Windows (even if I build with a Linux Docker image)
in case of shade transformer, thinking deeper at it, you are not forced to keep original newline: you can rewrite the file if you need it (and by "need", it can mean "makes your life easier"), the transformer perfectly knows the format of what it transforms, what is important is that:
1. the output is valid
2. the output is Reproducible, that is 2 runs give the same output (no randomness added, platform newline not being seen as randomness but reproducibility environment requirement = accepted constraint)
so choose the strategy you wish on newline for this file: it's a local personal choice that won't affect other parts of the build where platform newline is used.
IMHO only one choice is to be avoided: mix different newlines in one file. But even that one is a question of taste, because it does not break the 2 previous key requirements = valid + reproducible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] hboutemy edited a comment on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
hboutemy edited a comment on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-846365632
platform dependent newline is not an issue, it's normal, even if sometimes it adds some complexity: it does not block Reproducible Builds, see https://github.com/jvm-repo-rebuild/reproducible-central where I reproduced many releases done on Windows (even if I build with a Linux Docker image)
in case of shade transformer, thinking deeper at it, you are not forced to keep original newline: you can rewrite the file if you need it (and by "need", it can mean "makes your life easier"), the transformer perfectly knows the format of what it transforms, what is important is that:
1. the output is valid
2. the output is Reproducible, that is 2 runs give the same output (no randomness added, platform newline not being seen as randomness but reproducibility environment requirement = accepted constraint)
so choose the strategy you wish on newline for this file: it's a local personal choice that won't affect other parts of the build where platform newline is used.
IMHO only one choice is to be avoided: mix different newlines in one file. Even that one is a question of taste, because it does not break the 2 previous key requirements = valid + reproducible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas edited a comment on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas edited a comment on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-845654186
Bah, iirc the it enforces extra newline between merged segments, will fix. Or just the it uses plaf nelwine?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas commented on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-895236348
merged as c18d3d3e36f4d37aba98a18983bd4e56eba79327
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas closed pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas closed pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas commented on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-845689454
So, as I assumed: it is OS newline vs Java `\n`. But, the good news is that ONLY TWO classes in this project are affected by it (uses `System.getProperty( "line.separator" )`):
* the MavenJDOMWriter
* and the ServiceResourceTransformerTest UT that failed on CI. This is wrong IMO, as https://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html tells about file encoding, but IMO line ending of `\n` for it (on any plaf) is acceptable.
All the rest of code use `\n`.
UT is really unimportant (will align it with this PR), but unsure why MavenJDOMWriter uses OS newline, when IMHO it should be either using "edited POM" lineending or universal `\n`.
@elharo @michael-o any opinion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas commented on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-845654186
Bah, iirc the it enforces extra newline between merged segments, will fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] cstamas commented on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-845732594
@hboutemy expecting your response as well, as EOL thing affects reproducible builds.... (as same git hash on win or non-win produces different "edited" POM)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-shade-plugin] hboutemy commented on pull request #93: Improve and simplify ServicesResourceTransformer
Posted by GitBox <gi...@apache.org>.
hboutemy commented on pull request #93:
URL: https://github.com/apache/maven-shade-plugin/pull/93#issuecomment-846365632
platform dependent newline is not an issue, it's normal, even if sometimes it adds some complexity: it does not block Reproducible Builds, see https://github.com/jvm-repo-rebuild/reproducible-central where I reproduced many releases done on Windows (even if I build with a Linux Docker image)
in case of shade transformer, thinking deeper at it, you are not forced to keep original newline: you can rewrite the file if you need it (and by "need", it can mean "makes your life easier"), the transformer perfectly knows the format of what it transforms, what is important is that:
1. the output is valid
2. the output is Reproducible, that is 2 runs give the same output (no randomness added, newline not being seen as randomness but reproducibility environment requirement = accepted constraint)
so choose the strategy you wish on newline for this file: it's a local personal choice that won't affect other parts of the build where platform newline is used.
IMHO only one choice is to be avoided: mix different newlines in one file. Even that one is a question of taste, because it does not break the 2 previous key requirements = valid + reproducible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org