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

Change source encoding to UTF-8 #389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mkoncek
Copy link

@mkoncek mkoncek commented Jun 5, 2023

We have a project which bootstraps Maven. It manually calls javac and we encountered a problem with source encoding.
I would like to unify source encodings to UTF-8.

@garydgregory
Copy link
Member

Hello @mkoncek
How can this goal be enforced such that this build fails without the change?

@@ -297,7 +297,7 @@ private void addPaxHeadersForBigNumbers(final Map<String, String> paxHeaders,
TarConstants.MAXID);
// libarchive extensions
addFileTimePaxHeader(paxHeaders, "LIBARCHIVE.creationtime", entry.getCreationTime());
// star extensions by J�rg Schilling
// star extensions by Jörg Schilling
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also use:

// star extensions by Joerg Schilling

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, strictly (legally) speaking, "Jörg" and "Joerg" are not the same name, just like "Möller" and "Moeller" are not. So better stick to the original for credits / Copyrights, if possible.

@mkoncek
Copy link
Author

mkoncek commented Jun 5, 2023

maven-compiler-plugin is declared in commons-parent which also uses the encoding field. I rebased the PR so that the build reports an error the same way as javac would in our case. I don't know why the build doesn't fail, but there is an [ERROR] in the log if encoding is set and current sources are used.

commons-parent uses ISO-8859 encoding by default.

@sebbASF That would work too, but I believe it is time we can afford such luxury as using non-ASCII-only characters in sources.

@sebbASF
Copy link
Contributor

sebbASF commented Jun 5, 2023

I can confirm that changing the encoding causes the compile to report an ERROR, but the build succeeds:

$ mvn clean compile -Dcommons.encoding=UTF8
...
[INFO] Compiling 399 source files with javac [debug release 8] to target/classes
[ERROR] /commons/compress/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java:[300,32] unmappable character (0xF6) for encoding UTF-8
...
[INFO] BUILD SUCCESS

I've tried experimenting with -Dmaven.compiler.failOnWarning=true (and failOnError), but Maven does not fail the build.

However, adding -Dcommons.compiler.fork=true does cause the build to fail.
Possible bug in Maven?

@sebbASF
Copy link
Contributor

sebbASF commented Jun 6, 2023

@sebbASF
Copy link
Contributor

sebbASF commented Jun 6, 2023

Unfortunately, when using fork=true, some informational messages are not shown, see:
https://issues.apache.org/jira/browse/MCOMPILER-537

@sschuberth
Copy link
Contributor

How can this goal be enforced such that this build fails without the change?

Maybe by adding a UTF-8 encoded test asset, plus a test that reads the file in binary mode and compares to the expected bytes?

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

Successfully merging this pull request may close these issues.

4 participants