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

Runners do not respect COURSIER_REPOSITORIES environment variable with e.g. -213 #25

Closed
SethTisue opened this issue Sep 16, 2020 · 16 comments

Comments

@SethTisue
Copy link
Collaborator

SethTisue commented Sep 16, 2020

I have the variable set as follows, because I like to resolve everything through my local Artifactory whenever possible.

% echo $COURSIER_REPOSITORIES
ivy2Local|http://127.0.0.1:8081/artifactory/scala-ci-virtual/

This is my preference, but it might be a hard constraint for some users in corporate environments.

Anyway, the variable causes e.g. cs fetch to behave as follows:

% cs fetch org.scala-lang:scala-compiler:2.11.3
http://127.0.0.1:8081/artifactory/scala-ci-virtual/org/scala-lang/scala-compiler/2.11.3/scala-compiler-2.11.3.jar
  100.0% [##########] 13.0 MiB (203.0 MiB / s)

but scala-runners ignores the setting and goes to Maven Central:

% scala --scala-version 2.11.3
https://repo1.maven.org/maven2/org/scala-lang/scala-library/2.11.3/scala-library-2.11.3.jar
   67.3% [######    ] 3.6 MiB (838.5 KiB / s)
https://repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.11.3/scala-reflect-2.11.3.jar
   55.8% [#####     ] 2.3 MiB (578.1 KiB / s)
https://repo1.maven.org/maven2/org/scala-lang/scala-compiler/2.11.3/scala-compiler-2.11.3.jar
   21.8% [##        ] 2.8 MiB (683.7 KiB / s)
@dwijnand
Copy link
Owner

dwijnand commented Sep 17, 2020

scala-runners uses cs fetch (inspectable via scala -v --scala-version 2.11.3) so it looks like cs fetch doesn't honour COURSIER_REPOSITORIES. Do you want to look for or open an issue in Coursier for that?

@SethTisue
Copy link
Collaborator Author

SethTisue commented Sep 17, 2020

I think you must mean cs launch?

When I use -v as you suggest, the cs launch command line I see includes this line:

-r=central

so that looks like the cause. It's coming from this line in scala-runner:

    *+)                     addCoursier "--no-default" && addCoursier "-r=central" ;;

@dwijnand
Copy link
Owner

Yeah, sorry, cs launch, but I don't see a -r=central and I wouldn't expect one:

$ scala -v --scala-version 2.11.3
[setScalaVersion] arg = '2.11.3'
# Executing command line:
cs
launch
scala:2.11.3
--

Failed to created JLineReader: java.lang.NoClassDefFoundError: jline/console/completer/Completer
Falling back to SimpleReader.
Welcome to Scala version 2.11.3 (OpenJDK 64-Bit Server VM, Java 11.0.8).
Type in expressions to have them evaluated.
Type :help for more information.

@SethTisue
Copy link
Collaborator Author

ah, ok, there was a subtlety I missed here, which is that (due to multiple levels of shell aliasing) I was actually passing both -213 and --scala-version, which doesn't make much sense

and it's the -213 that's causing -r=central to be added

I'll tweak the issue title

@SethTisue SethTisue changed the title Runners do not respect COURSIER_REPOSITORIES environment variable Runners do not respect COURSIER_REPOSITORIES environment variable with e.g. -213 Sep 17, 2020
@dwijnand
Copy link
Owner

dwijnand commented Sep 17, 2020

That -r=central comes from #10, which I wasn't super-keen to deal with (at the scala-runners level, I mean). In my eyes (confirmation bias?) this is the fallout: my guess is -r overrides COURSIER_REPOSITORIES.

So... what to do? 😕

@dwijnand
Copy link
Owner

Or maybe it's the --no-default that makes it be ignored, but that and -r=central come together in scala-runners.

@dwijnand
Copy link
Owner

So here's a question. You mentioned your COURSIER_REPOSITORIES is:

% echo $COURSIER_REPOSITORIES
ivy2Local|http://127.0.0.1:8081/artifactory/scala-ci-virtual/

Which includes ivy2 local, which was the cause of #10. So, with:

  1. that environment variable set
  2. a locally published version of "2.13.4-bin-..." from a few weeks ago

what should happen when you run scala -213 (which launches scala:2.13+)? Should the setting of COURSIER_REPOSITORIES make it so it launches the (stale) 2.13.4-bin... version, or should it be ignored and 2.13.3 launched instead? I'm going to rule out parsing COURSIER_REPOSITORIES (even though it looks like a simple parse to do).

@SethTisue
Copy link
Collaborator Author

IMO -213 should run only full releases, not prereleases (-bin- and -pre), so yeah I'm expecting 2.13.3. (Whether Coursier provides any facility for doing such filtering, I don't know.)

@dwijnand
Copy link
Owner

Yeah Coursier has latest.stable, latest.release, latest.integration, and the + suffix. So in absence of anything in Coursier (or switching to a programmatic usage of Coursier - e.g. porting to Scala + native-image #19) what to do... Reverting #10 would fix this...

@SethTisue
Copy link
Collaborator Author

SethTisue commented Sep 30, 2020

Thought: could #10 be changed not to use -r, but to set COURSIER_REPOSITORIES instead, but only if the user hasn't already set it?

(It's possible I'm a bit confused.... sometimes these slow-motion discussions are hard to track.)

@dwijnand
Copy link
Owner

dwijnand commented Oct 1, 2020

That would mean that -213 would you'd get the 2.13.4-bin-... you published to ivy2 local 3 weeks ago (and forgot about) rather than 2.13.3. If you're happy with that I can give that a go.

@SethTisue
Copy link
Collaborator Author

SethTisue commented Oct 12, 2020

That would mean that -213 would you'd get the 2.13.4-bin-... you published to ivy2 local 3 weeks ago (and forgot about) rather than 2.13.3. If you're happy with that I can give that a go.

That sounds like a threat :-)

Your description is a bit grim. Under my suggestion, the dismal outcome you describe would only occur for people who explicitly set COURSIER_REPOSITORIES, which isn't most users.

@lrytz did you understand that when you thumbs-downed?

@lrytz
Copy link
Collaborator

lrytz commented Oct 16, 2020

Ah, no I didn't. Not sure what's more important, or how -213 is "specified", but I find it natural to assume that -213 runs the latest official release no matter what your env is. So I might prefer the status quo.

@SethTisue
Copy link
Collaborator Author

The change I'm suggesting only affects users of COURSIER_REPOSITORIES, which is only me, so I guess that means there are no actual objections then :-)

@dwijnand
Copy link
Owner

Got excepted by the notes in https://github.com/coursier/coursier/releases/tag/v2.0.10, but it turns out to just introduce ! as a shortcut for --no-defaults which we already use. I guess the problem is we don't have a specifier for stable releases, aka non -SNAPSHOT or -pre/-bin releases.

@SethTisue
Copy link
Collaborator Author

I don't have a local Artifactory anymore, so I wouldn't mind if you simply closed this.

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

No branches or pull requests

3 participants