Skip to content

XIVY-16828 Process issues not reported as [WARNING] #700

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

Merged
merged 2 commits into from
May 23, 2025
Merged

Conversation

ivy-rew
Copy link
Member

@ivy-rew ivy-rew commented May 23, 2025

enables our configuration for [WARNING] prefixed brackets, just like maven does, .... while the default of slf4j is [WARN].

since slf4j 2.x half-way removed that feature, we have to downgrade to 1.7.x:

  • maven CLI also uses slf4j.simpl 1.7
  • Nevertheless, I'll try to discuss the re-introduction of the 'warningString' configuration on the slf4j repos. Since the configuration values are still documented and in the code, but they are no longer interpreted 🤷

maven39-slf4j1

@ivy-rew ivy-rew changed the title reproduce: warnings are propagated to Maven CLI console XIVY-16828 warnings are propagated to Maven CLI console May 23, 2025
@ivy-rew ivy-rew marked this pull request as ready for review May 23, 2025 07:53
@ivy-rew
Copy link
Member Author

ivy-rew commented May 23, 2025

note that log4j-over-slf4j1 still works in combination with our log4j2 engine.
I didn't fully understand why, maybe because of the legacy12x api layer we have?
However, we should not be vulnerable to log4-shell, so I think there's nothing to worry about.
https://www.slf4j.org/log4shell.html
slf4j-api

@ivy-rew
Copy link
Member Author

ivy-rew commented May 23, 2025

Here's evidence, that the fix works: https://jenkins.ivyteam.io/job/project-build-plugin/job/mavenWarn2/7/maven-warnings/

However, since we have broken that feature a long time ago ... I'll merge it down to all affected LTS versions.
deprecation-warning

@ivy-rew ivy-rew requested a review from alexsuter May 23, 2025 08:05
Base automatically changed from slf4j.props to master May 23, 2025 08:22
@ivy-rew ivy-rew requested a review from ivy-cst May 23, 2025 11:17
Copy link
Member

@ivy-cst ivy-cst left a comment

Choose a reason for hiding this comment

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

Strange!
So we release a new project-plugin version 13.1.0.1?

@ivy-rew
Copy link
Member Author

ivy-rew commented May 23, 2025

Strange! So we release a new project-plugin version 13.1.0.1?

I wouldn't break the pattern; up to now just major and minor reflects the compatible release train.
Same pattern is used in the market environment.

@ivy-rew ivy-rew added the bug label May 23, 2025
@ivy-rew ivy-rew changed the title XIVY-16828 warnings are propagated to Maven CLI console XIVY-16828 Propagate Process warnings to Maven CLI console May 23, 2025
@ivy-rew ivy-rew changed the title XIVY-16828 Propagate Process warnings to Maven CLI console XIVY-16828 Process issues not reported as [WARNING] May 23, 2025
@ivy-rew ivy-rew merged commit f67a372 into master May 23, 2025
10 checks passed
@ivy-rew ivy-rew deleted the slf4j.warnings branch May 23, 2025 12:18
@ivy-cst
Copy link
Member

ivy-cst commented May 23, 2025

Strange! So we release a new project-plugin version 13.1.0.1?

I wouldn't break the pattern; up to now just major and minor reflects the compatible release train. Same pattern is used in the market environment.

When did we decide that?

@ivy-rew
Copy link
Member Author

ivy-rew commented May 23, 2025

Strange! So we release a new project-plugin version 13.1.0.1?

I wouldn't break the pattern; up to now just major and minor reflects the compatible release train. Same pattern is used in the market environment.

When did we decide that?

A decade before we started to do ADR's 😉
At any rate I don't feel we should introduce the first 4 digit version without discussing the pro's and con's.
So, if you want to change it, I think you should propose an ADR.

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

Successfully merging this pull request may close these issues.

2 participants