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

[Feature Request] Better output for mill show #2880

Closed
Avimitin opened this issue Nov 17, 2023 · 5 comments · Fixed by #2884
Closed

[Feature Request] Better output for mill show #2880

Avimitin opened this issue Nov 17, 2023 · 5 comments · Fixed by #2884
Milestone

Comments

@Avimitin
Copy link

Avimitin commented Nov 17, 2023

Currently our team is using mill to run a large number of build tasks, with each one returning the final binary path wrapped inside the PathRef type. These binaries are used in the final main task. But because the amount of these binaries are too large, so we decide to use mill -i -j0 show _.<our_task_name> to compile the build tasks simultaneously and use the return JSON to index those file, then package them together for distribution.

And here are some problems we encounter during the mill show and parsing process:

  1. Using -j0 can help us build all the tasks using all cores, but it will also add a prefix #[<number>] to the stdout, and cause the output JSON to be invalid. Now, to get the valid JSON, we can only use regex expression to trim the prefix.
  2. Because we are trying to package these binaries, we run mill inside a clean chroot. So every time we rebuild the package, mill will write "Preparing Java runtime, this might take one or two minutes.." to stdout, which will also break the output JSON. Our workaround is to run mill -i resolve _ > /dev/null to do a warm-up.
  3. Using mill show on the PathRef type will return a string with path and other metadata, so we need split the string with ':' and get the last segment.

Here is the real build script we are using to make the mill show work as expect

image

Here is some of my idea for improving the mill show command:

  1. using stdout to print JSON can help bash command line processing but is not reliable, so can we have a new option (like -o) to let mill show write the JSON output to a file?
  2. Is it possible to let PathRef output path only?
@lihaoyi
Copy link
Member

lihaoyi commented Nov 19, 2023

Is it possible to let PathRef output path only?

I think this is probably unlikely to be supported. mill show is meant to show the "raw" metadata, rather than any prettified version of it, and splitting it on a : is a small enough hardship in any JSON-processing environment that I don't think it's worth special casing in Mill here. Even jq has | split(":").

If you really want the raw paths from Mill, you can write your own target def yourTaskNamePath = T{ yourTaskName().path }, and show that to get them

We should fix everything else you brought up though. show output is meant to be clean. The fact that #[thread-id] and Preparing Java runtime are leaking into the show output is definitely not intended

@lihaoyi
Copy link
Member

lihaoyi commented Nov 19, 2023

Prefixing of show output should be fixed by #2884

w.r.t. "Preparing Java runtime, this might take one or two minutes..", I haven't managed to reproduce it being printed to stdout. AFAICT, it goes to stderr

logger.errorStream.println(
. @Avimitin do you have a minimal repro of it going to the stdout stream?

@Avimitin
Copy link
Author

Avimitin commented Nov 19, 2023

Yes, just run:

$ rm -r ~/.mill
$ mill -i -j0 show anything > stdout.txt
1 targets failed
show Cannot resolve anything. Try `mill resolve _` to see what's available.
$ cat stdout.txt
Preparing Java 21 runtime; this may take a minute or two ...

Also some additional version information:

$ mill --version
Mill Build Tool version 0.11.5
Java version: 21, vendor: N/A, runtime: /nix/store/q2vb58zhpsyhs3zq3zgyd3arv1m7b486-openjdk-21+35/lib/openjdk
Default locale: en_US, platform encoding: UTF-8
OS name: "Linux", version: 6.5.9-zen2-1-zen, arch: amd64

@lihaoyi
Copy link
Member

lihaoyi commented Nov 19, 2023

@Avimitin got it! Looks like @lefou already fixed that in #2839, which is scheduled to go out in 0.11.6. So once #2884 lands, that's probably it for the actionable improvements on this ticket

@Avimitin
Copy link
Author

Thanks!

@lefou lefou linked a pull request Nov 19, 2023 that will close this issue
@lefou lefou added this to the 0.11.6 milestone Nov 19, 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 a pull request may close this issue.

3 participants