SEPURI-SAI-KRISHNA opened a new pull request, #12735:
URL: https://github.com/apache/maven/pull/12735
### What
Uses `Locale.ROOT` in the three places where profile activation condition
evaluation was
implicitly relying on the JVM default locale:
- `ConditionFunctions.upper(..)` — `toUpperCase()` →
`toUpperCase(Locale.ROOT)`
- `ConditionFunctions.lower(..)` — `toLowerCase()` →
`toLowerCase(Locale.ROOT)`
- `ConditionParser.toString(..)` — `String.format("%.0f", ..)` →
`String.format(Locale.ROOT, "%.0f", ..)`
### Why
Profile activation should not depend on the locale of the machine running
the build, but today
it does. Turkish and Azeri map `i` to the dotted `İ` and `I` to the dotless
`ı`, so this profile:
```xml
<profile>
<activation>
<condition>upper(${os.name}) == 'WINDOWS'</condition>
</activation>
</profile>
```
silently fails to activate on a JVM started with `-Duser.language=tr`,
because `upper('windows')`
returns `WİNDOWS`. Reproduction:
```
$ jshell -R-Duser.language=tr
jshell> "windows".toUpperCase()
$1 ==> "WİNDOWS"
jshell> "LINUX".toLowerCase()
$2 ==> "lınux"
```
`ConditionParser.toString(Object)` has the same class of problem: it formats
whole doubles with
`String.format("%.0f", ..)` and no `Locale`, so under a locale whose default
numbering system is
not latin (e.g. `hi-IN-u-nu-deva`) it renders `42` as `४२`.
This brings the three call sites in line with the rest of the codebase,
which already passes an
explicit `Locale` in ~41 places — including `ExecutableFinder`, in this same
package.
### Why the existing tests missed it
Every string the current tests convert (`hello`, `WORLD`, `success`, `HELLO
WORLD`) happens to
contain no `i` or `I`, so `ConditionParserTest` passes unchanged under
`-Duser.language=tr`.
### Tests
Two tests added to `ConditionParserTest`, both of which fail without the
production change:
- `testCaseConversionFunctionsAreLocaleIndependent` — exercises
`upper`/`lower` under `en` and `tr`
- `testNumberFormattingIsLocaleIndependent` — exercises number rendering
under `hi-IN-u-nu-deva`
They follow the existing save/restore pattern from
`VersionTest.testCaseInsensitiveOrderingOfQualifiersIsLocaleIndependent` in
the same module.
Reverting only the production change and re-running gives:
```
[ERROR] ConditionParserTest.testCaseConversionFunctionsAreLocaleIndependent
upper() in tr ==> expected: <WINDOWS> but was: <WİNDOWS>
[ERROR] ConditionParserTest.testNumberFormattingIsLocaleIndependent
expected: <42> but was: <४२>
```
With the change, the full `maven-impl` module suite passes: 552 tests, 0
failures, checkstyle /
spotless / rat clean.
If this is wanted on `maven-4.0.x` as well, happy for it to be backported.
---
## PR checklist
The template ships with this checklist. Tick these:
- [x] Your pull request should address just one issue, without pulling in
other changes.
- [x] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject line
and body.
- [x] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied.
- [x] Run `mvn verify` to make sure basic checks pass.
- [x] I hereby declare this contribution to be licenced under the Apache
License Version 2.0, January 2004
Leave UNTICKED unless you actually run it (see note below):
- [ ] You have run the Core IT successfully.
The ICLA line can stay unticked — the production change is ~20 lines, under
the template's
"~20 lines of code" threshold.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
