-
Notifications
You must be signed in to change notification settings - Fork 100
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
Resolves #152 - Adds option to redirect program output of exec:exec to the maven logger. #153
Conversation
{ | ||
getLog().debug("Will redirect program output to Maven logger"); | ||
final String parentThreadName = Thread.currentThread().getName(); | ||
final String logSuffix = "[" + parentThreadName + "] "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more of a prefix instead of a suffix… but it always says [main]
for me and as such is redundant and unnecessary in the output (makes it unnecessarily wider)…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Jenkins, they are [pool-1-thread-1 for channel]
, making the output definitely too wide to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mirabilos Good point about prefix instead of suffix, my bad.
As for always getting [main]
, were you running Maven multi-threaded on a reactor that is bound to have 2 projects running in parallel with this plugin? This option adds no value for single threaded builds, but can become an aid in tracing program output in large reactors with lots of threads. For example 1000+ projects running with 50+ threads.
Daniel Johnson dixit:
@mirabilos Good point about prefix instead of suffix, my bad.
np
As for always getting `[main]`, were you running Maven multi-threaded
Nope, just “mvn clean install” on the commandline, or, worse,
in Jenkins.
on a reactor that is bound to have 2 projects running in parallel with
this plugin? This option adds no value for single threaded builds, but
can become an aid in tracing program output in large reactors with lots
of threads. For example 1000+ projects running with 50+ threads.
Yes, I understand. But for single-threaded execution it’s rather
redundant and annoying.
Is there a way to detect whether it runs multi-threaded, so it
could disable the prefix if not? Otherwise, I’d also be happy
with an option (one could set that on the command line or in
the Jenkins configuration if things always run single-threaded,
or in the POM if one expects things to do so).
Thanks,
//mirabilos
--
(gnutls can also be used, but if you are compiling lynx for your own use,
there is no reason to consider using that package)
-- Thomas E. Dickey on the Lynx mailing list, about OpenSSL
|
This new |
Daniel Johnson dixit:
This new `useMavenLogger` option itself _should_ be disabled by
It is.
default. If your environment is using single threaded builds then
simply don't enable this new option to stick with the old behavior. My
Hm, but it prefixes the output lines, which _is_ helpful, and
so I just rewrote my scripts to make use of it. Plus it prepares
for parallel execution.
expectation for this property is that it would be enabled on command
line at the same time multi-threaded builds are enabled, for example:
No, this won’t work at all, because normal Unix use of stderr
is incompatible with the useMavenLogger concept of using it for
a loglevel (in Unix normally everything interactive goes to
stderr, *especially* informative and progress notices, and some
scripts use concepts like…
for file in 1 2 3; do
exec >$file
produce_output
done
exec >&2
… to get a working-enough stdout back after temporarily closing
it; I had to completely rework I/O in my scripts and ended up
duplicating stdout to fd#8 and stderr to #9 early and then make
sure I output info and errors to these two only).
It doesn't add value for local builds of single projects, and is more
useful for CI/CD builds.
Perhaps, besides the helpful prefixing, but, as I said, the
scripts themselves must be (re)written to make use of it, so
it’s not a simple toggle.
Full disclosure: I’m the developer of a Unix shell (the most
widespread in use one, in fact, given it’s also used on two+
billion Android devices as /system/bin/sh) and have over two
decades of shell experience alone.
bye,
//mirabilos
--
„Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
mksh auf jedem System zu installieren.“
-- XTaran auf der OpenRheinRuhr, ganz begeistert
(EN: “[…]uhr.gz is a reason to install mksh on every system.”)
|
Hi @mirabilos - I created Pull Request #157 to at least address your concern to not print the thread name prefix when Maven itself is not running with multiple threads. Your points above about how scripts tend to use stderr/stdout I am sure are completely valid, but I am not a shell power user, so it is well beyond my limited understanding. If you find this solution is not honoring the original intention behind stderr/stdout (or how these are most commonly used by scripts) I am not at all offended if you propose a patch to these changes that more closely aligns with them. Cheers! |
Daniel Johnson dixit:
Hi @mirabilos - I created Pull Request #157 to at least address your
concern to not print the thread name prefix when Maven itself is not
running with multiple threads.
Aweseome, thanks!
Your points above about how scripts tend to use stderr/stdout I am sure
are completely valid, but I am not a shell power user, so it is well
beyond my limited understanding. If you find this solution is not
honoring the original intention behind stderr/stdout (or how these are
most commonly used by scripts) I am not at all offended if you propose
a patch to these changes that more closely aligns with them.
This is really tricky. I originally wrote my scripts to use Maven-like
præficēs ([INFO], [WARNING], [ERROR]) for… most lines, at least.
I think your proposal isn’t bad, it’s just not something you’d
simply enable/disable with a switch without the script being
prepared for it (e.g. writing to stderr without an extra error
prefix in the nōn-mavenLogger case would cause a user to possibly
miss it).
So I’d say leave it as-is. It’s still a good idea.
Thank you,
//mirabilos
--
> Why don't you use JavaScript? I also don't like enabling JavaScript in
Because I use lynx as browser.
+1
-- Octavio Alvarez, me and ⡍⠁⠗⠊⠕ (Mario Lang) on debian-devel
|
These changes would add two new options to exec:exec goal:
exec.outputFile
is specified.exec.useMavenLogger=true
, prints all executed program output at debug level instead of the default info level to the Maven logger.Please see the javadocs in the code for more information.