Skip to content

scala-runners: init at 0c0b369#84068

Closed
hrhino wants to merge 2 commits intoNixOS:masterfrom
hrhino:scala-runners
Closed

scala-runners: init at 0c0b369#84068
hrhino wants to merge 2 commits intoNixOS:masterfrom
hrhino:scala-runners

Conversation

@hrhino
Copy link
Copy Markdown
Contributor

@hrhino hrhino commented Apr 2, 2020

This is a script to run multiple versions of scala either by version number or build hash.

Motivation for this change

It's a good script and useful for testing with pre-release and non-current versions of Scala.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS <-- don't have one!
    • other Linux distributions <-- don't have any but can if you like
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests) <-- manually tested this; coursier and jdk are the only upstreams
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • 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) <-- Total closure size is 390.4Mb but almost all of that is openjdk and coursier
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Apr 2, 2020
Copy link
Copy Markdown
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Tested on NixOS, compiling and running a Scala program on different versions indeed works.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the convention is to put this into a separate commit titled maintainers: add hrhino (added #96666)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done; thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also pushed a newer version because, why not.

@hrhino hrhino force-pushed the scala-runners branch 2 times, most recently from 6022af9 to 975ba3f Compare September 13, 2020 18:26
@hrhino hrhino changed the title scala-runners: init at e6996f8 scala-runners: init at 0c0b369 Sep 13, 2020
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let
let

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yikes. thanks.

Comment on lines 3 to 7
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let
name = "scala-runners";
owner = "dwijnand";
in stdenv.mkDerivation {
inherit name;
stdenv.mkDerivation rec {
name = "scala-runners";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, instead of having just a name, packages should nowadays have a pname and a version (#103997)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@raboof should I use (part of) the git tag? I can bother Dale if he wants to push tags but I think he may not care.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there's no clear consensus on how to version packages where upstream doesn't use tags (#100833), using the (short) git tag seems ok to me in this case.

Copy link
Copy Markdown
Member

@AndersonTorres AndersonTorres Feb 16, 2021

Choose a reason for hiding this comment

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

@hrhino the convention I follow and I think is the best way to deal is

pname = "<name of software>";
version = "unstable-${date of the commit in the format YYYY-MM-DD}";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please put meta at the end.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done; thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
homepage = "https://github.com/${owner}/${name}";
homepage = "https://github.com/dwijnand/scala-runners";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done; thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
maintainers = [ maintainers.hrhino ];
maintainers = with maintainers; [ hrhino ];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
inherit owner;
owner = "dwijnand";

Comment on lines 24 to 35
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
installPhase = ''
set -x
mkdir -p $out/bin $out/lib
sed -ie "s_\Wcs\W_ ${coursier}/bin/coursier _" scala-runner
cp scala-runner $out/lib
ln -s $out/lib/scala-runner $out/bin/scala
ln -s $out/lib/scala-runner $out/bin/scalac
ln -s $out/lib/scala-runner $out/bin/scalap
ln -s $out/lib/scala-runner $out/bin/scaladoc
'';
installPhase = ''
mkdir -p $out/bin $out/lib
sed -ie "s_\Wcs\W_ ${coursier}/bin/coursier _" scala-runner
cp scala-runner $out/lib
ln -s $out/lib/scala-runner $out/bin/scala
ln -s $out/lib/scala-runner $out/bin/scalac
ln -s $out/lib/scala-runner $out/bin/scalap
ln -s $out/lib/scala-runner $out/bin/scaladoc
'';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what changed here? just removing the set -x? I guess that's a good idea.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah. not sure why I selected more lines.

@SuperSandro2000 SuperSandro2000 marked this pull request as draft December 27, 2020 23:56
This is a script to run multiple versions of scala either by version
number or build hash.
Comment on lines +3 to +4
let
in stdenv.mkDerivation rec {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
let
in stdenv.mkDerivation rec {
stdenv.mkDerivation rec {


let
in stdenv.mkDerivation rec {
name = "scala-runners";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
name = "scala-runners";
pname = "scala-runners";
version = "unstable-2020-02-02"


scala = scala_2_13;
scala-runners = callPackage ../development/compilers/scala-runners/default.nix {
coursier = callPackage ../development/tools/coursier { jre = jdk8; };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
coursier = callPackage ../development/tools/coursier { jre = jdk8; };
coursier = coursier.override { jre = jdk8; };

@raboof raboof self-requested a review May 1, 2021 18:34
@hrhino hrhino closed this May 26, 2021
@AndersonTorres
Copy link
Copy Markdown
Member

?

@hrhino
Copy link
Copy Markdown
Contributor Author

hrhino commented Jun 25, 2021

#128060 to replace this because apparently force-push then reopen doesn't work. yay, github.

@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants