Add mill build tool #51110

scalavision · 2018-11-27T12:51:05Z

Motivation for this change

Mill is a very nice tool for building java and scala projects. It is easier to learn than sbt, maven and gradle for beginners in Scala.

I can be the maintainer of this package, I am also working on several new bioinformatic tools that I will add shortly to nixpkgs. I am still learning nix so please let me know what could be improved! I have mostly looked at other, similar projects to accomplish this derivation.

Thank you for creating Nix and NixOS, it's an awesome project!

Things done

[x ] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- [x ] NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
[ x] Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
[x ] Assured whether relevant documentation is up to date
[ x] Fits CONTRIBUTING.md.

alyssais · 2018-11-27T12:52:57Z

pkgs/development/tools/build-managers/mill/default.nix

+    ${gnused}/bin/sed -i '0,/mill.MillMain/{s|mill.MillMain|mill.MillMain --no-remote-logging|}' $out/bin/mill
+  '';
+   meta = with stdenv.lib; {
+    homepage = http://www.lihaoyi.com/mill;


Might as well be HTTPS?

alyssais · 2018-11-27T12:53:20Z

pkgs/development/tools/build-managers/mill/default.nix

+    cp ${src} $out/bin/mill
+    chmod +x $out/bin/mill
+    ${gnused}/bin/sed -i '0,/java/{s|java|${jre}/bin/java|}' $out/bin/mill
+    ${gnused}/bin/sed -i '0,/mill.MillMain/{s|mill.MillMain|mill.MillMain --no-remote-logging|}' $out/bin/mill


Do you need to manually do gnused like this? It should be in stdenv, shouldn't it?

alyssais · 2018-11-27T12:54:05Z

pkgs/development/tools/build-managers/mill/default.nix

+   propagatedBuildInputs = [ jre ] ;
+  buildInputs = [ makeWrapper gnused ] ;
+   phases = "installPhase";
+   installPhase = ''


Indentation seems to be a little wonky

alyssais · 2018-11-27T12:54:24Z

pkgs/development/tools/build-managers/mill/default.nix

+ stdenv.mkDerivation rec {
+  name = "mill-${version}";
+  version = "0.3.5";
+    src = fetchurl {


Use fetchFromGitHub here

Thanks for all your feedback @alyssais !

Lihaoyi has prebuilt the Mill binary on this url:

https://github.com/lihaoyi/mill/releases/download/0.3.5/0.3.5

I am not able to fetch that since it does not have the .tar.gz extension.

I am not sure how easy it is to build this from source, since it is a build tool that is building itself. Would it be ok to keep the fetchurl. Are there any special requirements concerning using binaries like this in nixpkgs?

I've used sbt that is also a build tool in scala as my recipe:

https://github.com/scalavision/nixpkgs/blob/add-mill-build-tool/pkgs/development/tools/build-managers/sbt/default.nix

Aha! I didn't catch that it was a binary. Generally builds from source are vastly preferred in Nixpkgs. Usually binaries are only used when source builds are impossible (eg if it's proprietary, closed-source software).

If it has a dependency on itself, something that, eg ghc and rustc do it to pull in a binary of the previous release, and use that to build the latest one from source.

Probably best to wait for somebody else to weigh in about the binary – personally I'd rather not have binaries in Nixpkgs when it's not absolutely necessary, but that's not my call.

Yes.. I agree about binaries :-) I think creating a build using scalac would be quite some work though. I'll wait to see. Thank you very much for helping me out so far! :-)

alyssais · 2018-11-27T12:55:47Z

pkgs/development/tools/build-managers/mill/default.nix

+     sha256 = "19ka81f6vjr85gd8cadn0fv0i0qcdspx2skslfksklxdxs2gasf8";
+  };
+   propagatedBuildInputs = [ jre ] ;
+  buildInputs = [ makeWrapper gnused ] ;


Where do you use makeWrapper?

alyssais · 2018-11-27T12:56:18Z

pkgs/development/tools/build-managers/mill/default.nix

+    ${gnused}/bin/sed -i '0,/java/{s|java|${jre}/bin/java|}' $out/bin/mill
+    ${gnused}/bin/sed -i '0,/mill.MillMain/{s|mill.MillMain|mill.MillMain --no-remote-logging|}' $out/bin/mill
+  '';
+   meta = with stdenv.lib; {


You've taken lib as an argument so this can just be with lib.

alyssais · 2018-11-27T12:57:09Z

pkgs/development/tools/build-managers/mill/default.nix

+  };
+   propagatedBuildInputs = [ jre ] ;
+  buildInputs = [ makeWrapper gnused ] ;
+   phases = "installPhase";


What's this line for?

It is used to override all other phases with my own defined installPhase. This will crash on the unpack phase for example, since it is a binary. Maybe there are better ways doing this? I think I took this from the nix pills.

alyssais · 2018-11-27T12:57:32Z

pkgs/development/tools/build-managers/mill/default.nix

+   propagatedBuildInputs = [ jre ] ;
+  buildInputs = [ makeWrapper gnused ] ;
+   phases = "installPhase";
+   installPhase = ''


If you're overriding installPhase you should add runHook preInstall and runHook postInstall to your implementation.

Do I need to do that also if we only have our own self defined installPhase? I tested it locally and it works, but I guess that is because they are undefined?

Yeah, because it allows people to use overrideAttrs and insert their own things without having to override your whole installPhase.

of course! hehe, didn't think about that! Nice :-)

alyssais · 2018-11-27T12:59:11Z

We have a standard commit message format. In this case, it should be mill: init at 0.3.5.

…or pre- and postInstall

veprbl · 2018-11-27T20:56:11Z

pkgs/development/tools/build-managers/mill/default.nix

+
+  propagatedBuildInputs = [ jre ] ;
+
+  phases = "installPhase";


This is not a good way. You are throwing away the standard unpackPhase and fixupPhase.
I think its fine to instead let the standard phases fail here, they should not terminate the build. You can ask to skip some of the phases by setting

stdenv.mkDerivation { # ... dontConfigure = true; dontBuild = true; # ... }

Note that dontConfigure doesn't work at the moment, but it's worth to state the intent.

Thanks again for helping out on this. I've added the flags you mention, I also override the unpackPhase, so there are no errors occuring during the build. Is this an ok way of doing this? Without it, my build did actually stall locally.

I've also removed the sed patching, it was not necessary it seems.

…lags and unpackPhase override

veprbl · 2018-11-28T20:04:37Z

@scalavision
There are still few problems. Using ${src} will hardcode output path of fetchurl into your installPhase, then you can't override mill.overrideAttrs (_: { src = ...; }). The solution is to use $src which is substituted by bash from environment variable.

Other problem is that propagatedBuildInputs doesn't work for nix-env. The solution here is to use makeWrapper.

Finally, meta.platforms depends on what platforms your actual jre implementation can be built for.

I propose following fixes:

diff --git a/pkgs/development/tools/build-managers/mill/default.nix b/pkgs/development/tools/build-managers/mill/default.nix
index 933a7e56cca..97910d22cb3 100644
--- a/pkgs/development/tools/build-managers/mill/default.nix
+++ b/pkgs/development/tools/build-managers/mill/default.nix
@@ -1,4 +1,5 @@
-{ stdenv, fetchurl, jre }:
+{ stdenv, fetchurl, jre, makeWrapper }:
+
 stdenv.mkDerivation rec {
   name = "mill-${version}";
   version = "0.3.5";
@@ -8,11 +9,9 @@ stdenv.mkDerivation rec {
     sha256 = "19ka81f6vjr85gd8cadn0fv0i0qcdspx2skslfksklxdxs2gasf8";
   };

-  propagatedBuildInputs = [ jre ] ;
+  nativeBuildInputs = [ makeWrapper ];

-  unpackPhase = ''
-    # Skipping unpackphase, nothing to unpack
-  '';
+  unpackPhase = "true";

   dontConfigure = true;

@@ -20,9 +19,9 @@ stdenv.mkDerivation rec {

   installPhase = ''
     runHook preInstall
-    mkdir -p $out/bin
-    cp ${src} $out/bin/mill
-    chmod +x $out/bin/mill
+    install -Dm555 "$src" "$out/bin/.mill-wrapped"
+    # can't use wrapProgram because it sets --argv0
+    makeWrapper "$out/bin/.mill-wrapped" "$out/bin/mill" --prefix PATH : ${stdenv.lib.makeBinPath [ jre ]}
     runHook postInstall
   '';

@@ -31,7 +30,6 @@ stdenv.mkDerivation rec {
     license = licenses.mit;
     description = "A build tool for Scala, Java and more";
     maintainers = with maintainers; [ scalavision ];
-    platforms = platforms.unix;
+    inherit (jre.meta) platforms;
   };
-
 }

scalavision · 2018-11-29T13:03:50Z

Thank you very much for your feedback @veprbl :-)
I've learned a lot from this!!

scalavision · 2018-11-29T15:19:09Z

oh.. hm... do you think there is a way to solve it? I used this as my recipe originally, I see they have used patform.all in there.

https://github.com/NixOS/nixpkgs/blob/dd46b1adb6da5e98fa5850cef6a07c29e3659fa0/pkgs/development/tools/ammonite/default.nix

veprbl · 2018-11-29T15:38:44Z

platform.all should be fine, I think. It should raise a meaningful error if jre is not supported on the current platform.

veprbl · 2018-11-29T16:19:11Z

@GrahamcOfBorg build mill

GrahamcOfBorg · 2018-11-29T16:19:25Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: mill

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-11-29T16:19:57Z

Success on x86_64-darwin (full log)

Attempted: mill

Partial log (click to expand)

unpacking sources
patching sources
configuring
no configure script, doing nothing
installing
post-installation fixup
strip is /nix/store/mvpvjar6m4jpjcz48715w2pax53djv6g-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/4qjjdb2jfqkxhvsxcnjxqmlyxgjc5jb2-mill-0.3.5/bin
patching script interpreter paths in /nix/store/4qjjdb2jfqkxhvsxcnjxqmlyxgjc5jb2-mill-0.3.5
/nix/store/4qjjdb2jfqkxhvsxcnjxqmlyxgjc5jb2-mill-0.3.5

GrahamcOfBorg · 2018-11-29T16:20:41Z

Success on x86_64-linux (full log)

Attempted: mill

Partial log (click to expand)

configuring
no configure script, doing nothing
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/imc6ypa31lmzp1zlia8xikb82n0dkdb0-mill-0.3.5
strip is /nix/store/n4hb93w6j076xcjw5pm09rdmc09s075b-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/imc6ypa31lmzp1zlia8xikb82n0dkdb0-mill-0.3.5/bin
patching script interpreter paths in /nix/store/imc6ypa31lmzp1zlia8xikb82n0dkdb0-mill-0.3.5
checking for references to /build in /nix/store/imc6ypa31lmzp1zlia8xikb82n0dkdb0-mill-0.3.5...
/nix/store/imc6ypa31lmzp1zlia8xikb82n0dkdb0-mill-0.3.5

veprbl · 2018-11-29T16:53:18Z

Seems to work to compile and run a simple hello world Scala project. I don't understand where it gets java and scala compiler from. @scalavision do you understand how it works?

veprbl

Okay. I see that it does look for javac to compile java. It seems to work fine for interactive use as is. It probably will need some more work to be used inside a Nix build (e.g. some artefacts need to be fetched, probably can be done in a way similar to what's done in bloop).

I think this is good to be merged.
Just in case, cc'ing sbt maintainers @NeQuissimus @rickynils

scalavision · 2018-11-30T10:34:11Z

I am really no sure @veprbl , it seems he has a dependency on zinc, and maybe that one does the magic. I think he has reused some stuff from sbt and zinc.

https://github.com/sbt/zinc

I agree, I think support for use within a nix build could be added on later. Again, thank you very much for helping out :-)

scalavision added 2 commits November 27, 2018 13:20

adding mill to nixpkgs

79c2020

added maintainer

3e8a50d

alyssais requested changes Nov 27, 2018

View reviewed changes

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Nov 27, 2018

scalavision added 3 commits November 27, 2018 15:12

fixed all feedback from review except changing fetchurl and runHook f…

472bc90

…or pre- and postInstall

added accidently removed newline at the end of file

10d48b5

fixed indentation

5f504c6

veprbl reviewed Nov 27, 2018

View reviewed changes

scalavision added 2 commits November 28, 2018 11:45

removed installPhase override, mill binary sed patching, added dont f…

7a64c72

…lags and unpackPhase override

changed echo to comment in unpackPhase to avoid unessecary noise

780fc5e

update according to feedback

d524d19

GrahamcOfBorg added 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 1-10 labels Nov 29, 2018

changing to platformas.all, added longDescription

5c51ed9

GrahamcOfBorg added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 labels Nov 29, 2018

veprbl approved these changes Nov 29, 2018

View reviewed changes

veprbl merged commit e0e8486 into NixOS:master Dec 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mill build tool #51110

Add mill build tool #51110

scalavision commented Nov 27, 2018

alyssais Nov 27, 2018

alyssais Nov 27, 2018

alyssais Nov 27, 2018

alyssais Nov 27, 2018

scalavision Nov 27, 2018

alyssais Nov 27, 2018

scalavision Nov 27, 2018

alyssais Nov 27, 2018

alyssais Nov 27, 2018

alyssais Nov 27, 2018

scalavision Nov 27, 2018

alyssais Nov 27, 2018

scalavision Nov 27, 2018

alyssais Nov 27, 2018

scalavision Nov 27, 2018

alyssais commented Nov 27, 2018

veprbl Nov 27, 2018 •

edited

Loading

scalavision Nov 28, 2018

veprbl commented Nov 28, 2018

scalavision commented Nov 29, 2018

scalavision commented Nov 29, 2018

veprbl commented Nov 29, 2018

veprbl commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

veprbl commented Nov 29, 2018

veprbl left a comment

scalavision commented Nov 30, 2018

Add mill build tool #51110

Add mill build tool #51110

Conversation

scalavision commented Nov 27, 2018

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alyssais commented Nov 27, 2018

veprbl Nov 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veprbl commented Nov 28, 2018

scalavision commented Nov 29, 2018

scalavision commented Nov 29, 2018

veprbl commented Nov 29, 2018

veprbl commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

GrahamcOfBorg commented Nov 29, 2018

veprbl commented Nov 29, 2018

veprbl left a comment

Choose a reason for hiding this comment

scalavision commented Nov 30, 2018

veprbl Nov 27, 2018 •

edited

Loading