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

Add property 'mvnd.preserveProjectLog' to not decorate the stdout/stderr stream #726

Closed
wants to merge 1 commit into from

Conversation

gzm55
Copy link
Contributor

@gzm55 gzm55 commented Oct 21, 2022

With the new property set to true, the prefix [INFO] [stdout] is removed in the client terminal output. Then without --raw-stream, we can still get the same behavior with original maven.

@gzm55
Copy link
Contributor Author

gzm55 commented Jan 12, 2023

@gnodet @ppalaga what about this pr?

@ppalaga
Copy link
Contributor

ppalaga commented Jan 12, 2023

Could you please explain what are the possible motivations for this change? In which situations does it help? Why would user want to have this?

@ppalaga
Copy link
Contributor

ppalaga commented Jan 12, 2023

I must say I have hard time figuring out how this combines with --raw-streams. It sounds like you need some additional fine tuning of how the system streams are set up and which is currently not possible with --raw-streams. Would it not be better to deprecate --raw-streams and introduce a new param with more possibilities?

I am also a bit lost how we handle the system streams in the daemon. @gnodet why do we cal System.setOut() and System.setErr() on so many places?

@gzm55
Copy link
Contributor Author

gzm55 commented Jan 12, 2023

@ppalaga this option is used to keep the logging behavior the same as the original maven:

Expected from maven (no new-line char at the end):

$ mvn -DforceStdout -q help:evaluate -Dexpression=project.version -l >(sed 's/^/in log: /')
in log: 1.0.0

mvnd default:

$ mvnd -DforceStdout -q help:evaluate -Dexpression=project.version -l >(sed 's/^/in log: /')
in log: [INFO] [stdout] 1.0.0

mvnd with --raw-streams:

$ mvnd -DforceStdout -q help:evaluate -Dexpression=project.version -l >(sed 's/^/in log: /') --raw-streams
1.0.0

mvnd with new option -Dmvnd.preserveProjectLog (with a new-line char at the end):

$ mvnd -DforceStdout -q help:evaluate -Dexpression=project.version -l >(sed 's/^/in log: /') -Dmvnd.preserveProjectLog
in log: 1.0.0

@gnodet
Copy link
Contributor

gnodet commented Jan 23, 2023

Note that I have the following:

> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  -l >(sed 's/^/in log: /') 2>/dev/null
1.0.0-SNAPSHOT
> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  1> >(sed 's/^/in log: /') 2>/dev/null
in log: 1.0.0-SNAPSHOT

@gnodet
Copy link
Contributor

gnodet commented Jan 23, 2023

Note that I have the following:

> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  -l >(sed 's/^/in log: /') 2>/dev/null
1.0.0-SNAPSHOT
> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  1> >(sed 's/^/in log: /') 2>/dev/null
in log: 1.0.0-SNAPSHOT

The problem is that mvnd implements the -l switch in a different way than maven. When a -l switch is specified, maven sets the System.out and System.err streams to the file redirection. This option is not passed to maven by mvnd, and instead, it redirects the project output to the file, but does not touch the system out and err streams. Fixing that will lead to a behaviour which will be closer to the maven one.

@gnodet
Copy link
Contributor

gnodet commented Jan 23, 2023

Note that I have the following:

> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  -l >(sed 's/^/in log: /') 2>/dev/null
1.0.0-SNAPSHOT
> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  1> >(sed 's/^/in log: /') 2>/dev/null
in log: 1.0.0-SNAPSHOT

The problem is that mvnd implements the -l switch in a different way than maven. When a -l switch is specified, maven sets the System.out and System.err streams to the file redirection. This option is not passed to maven by mvnd, and instead, it redirects the project output to the file, but does not touch the system out and err streams. Fixing that will lead to a behaviour which will be closer to the maven one.

#779 is an attempt to fix the above problem. It now gives the correct result (apart the added terminal newline)

@gzm55
Copy link
Contributor Author

gzm55 commented Jan 23, 2023

Note that I have the following:

> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  -l >(sed 's/^/in log: /') 2>/dev/null
1.0.0-SNAPSHOT
> mvnd -q -DforceStdout help:evaluate -Dexpression=project.version --raw-streams  1> >(sed 's/^/in log: /') 2>/dev/null
in log: 1.0.0-SNAPSHOT

The problem is that mvnd implements the -l switch in a different way than maven. When a -l switch is specified, maven sets the System.out and System.err streams to the file redirection. This option is not passed to maven by mvnd, and instead, it redirects the project output to the file, but does not touch the system out and err streams. Fixing that will lead to a behaviour which will be closer to the maven one.

#779 is an attempt to fix the above problem. It now gives the correct result (apart the added terminal newline)

@gnodet the nearly same behavior of -l option in #779 should be better than this pr.

@gnodet
Copy link
Contributor

gnodet commented Jan 23, 2023

Superseded by #779

@gnodet gnodet closed this Jan 23, 2023
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.

3 participants