Remove wrapper script #11

dfornika · 2017-09-12T14:45:39Z

Addresses #10.

conda-forge-linter · 2017-09-12T14:45:45Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jakirkham · 2017-09-12T15:19:50Z

So I think this content needs to be moved to juliarc.jl. Should be in $PREFIX/etc/julia/juliarc.jl.

jakirkham · 2017-09-12T16:01:03Z

Tried to add this code for the juliarc instead of the wrapper script and it seems to work ok.

# $PREFIX/etc/julia/juliarc.jl

JULIA_PREFIX = abspath(joinpath(Base.source_path(), "..", "..", ".."))

if !("JULIA_PKGDIR" in keys(ENV))
    ENV["JULIA_PKGDIR"] = joinpath(JULIA_PREFIX, "share", "julia", "site")
    Pkg.init()
    Pkg.__init__()
    pop!(Base.LOAD_CACHE_PATH)
end

if !("JULIA_HISTORY" in keys(ENV))
    ENV["JULIA_HISTORY"] = joinpath(JULIA_PREFIX, ".julia_history")
end

jakirkham · 2017-09-12T16:27:34Z

Added PR ( dfornika#1 ) against your branch with this sort of change.

* Add a custom juliarc.jl Needed to ensure packages and other relevant information to the Julia install are tracked in the conda environment. * Install custom juliarc Make sure that that our custom juliarc is available in the standard location. * Create the package directory Make sure that Julia initializes our package directory and includes the standard packages in it. * Bump build number to 1 Needed to repackage after removal of the wrapper script, inclusion of juliarc, and inclusion of the prepopulated package directory.

jakirkham · 2017-09-12T16:58:49Z

recipe/juliarc.jl

+
+if !("JULIA_HISTORY" in keys(ENV))
+    ENV["JULIA_HISTORY"] = joinpath(JULIA_PREFIX, ".julia_history")
+end


So I know I added this and it just imitates the wrapper script that we are replacing, but I don't think it is a good idea. If there is a multiuser conda install, we don't want to squash their history into one file shared by all of them. This is bad for user's tracking their own history and is bad if some of them lack permissions to the conda install. Instead we should keep them separated per user. Ideally this is done by allowing this to write in the user's home directory somewhere. This is actually what we do with IPython as well.

TL;DR We should remove these 3 lines.

that applies to users' packages too doesn't it?

That is a totally different view of package management from the one conda is based in. The point of using something like conda is to allow for multiple different independent environments where many different versions of packages and their dependencies live. This allows one to easily switch between different development and production environments trivially on a regular basis. Also users can trivially toss aside an environment or create a new one fresh without concerns of other dependencies leaking in. We can't very easily do any of these things if packages live outside the environment.

Should add that one can create conda environments anywhere. So if a user is working with a multiuser conda installation and does not have permissions to the multiuser environments (or wishes not to mess with them), they still can create an environment in their home directory or anywhere else with packages of their choosing.

won't that force them to reinstall julia? packages are user data, not part of the language, and shouldn't be forced to be installed in the same place

It won't cache julia packages.

Sorry, what won't?

I don't think you should put user-installed julia packages under the language install tree.

Ok. Should we be using local or something to disambiguate? They are under site now, which doesn't overlap with base.

conda won't cache julia packages if they're placed as extra content inside a different installed package (the julia language)

most julia users would expect packages to be shared between different methods of installing the same minor version of julia. if conda starts managing julia packages (I'd advise against this until after the julia package manager has been rewritten) then you could add them to the LOAD_PATH without involving julia's package manager at all.

conda won't cache julia packages if they're placed as extra content inside a different installed package (the julia language)

Right, but the plan would be to package Julia packages as conda packages. At which point conda would be able to handle them like any other conda package.

most julia users would expect packages to be shared between different methods of installing the same minor version of julia.

This will be possible as we add conda packages of julia packages.

if conda starts managing julia packages (I'd advise against this until after the julia package manager has been rewritten) then you could add them to the LOAD_PATH without involving julia's package manager at all.

Ok noted.

I think this discussion has diverged quite a bit from the original PR, which is my fault. We should move this discussion about how to manage Julia packages within the context of conda into a new issue so that we can discuss this further. Would hate to have this discussion buried in outdated comment of an unrelated diff.

Both ways - a non-conda install of julia should be able to see packages added by running Pkg.add within a conda-installed julia. It might not see Julia packages added via conda install, but I don't think you should go there.

Have open issue ( #14 ) to discuss how best to manage Julia packages with conda. TBH this is something we are going to want to hammer out sooner rather than later. So we should keep an eye open for practical near term solutions.

…dstock into remove-wrapper-script

jakirkham · 2017-09-12T18:10:38Z

recipe/build.sh


+# Populate initial package directory
+julia -e "Pkg.init(); Pkg.__init__()"


We may need to drop Pkg initialization step after all. Appears conda-build strips out .git directories, which will break the METADATA repo. There are some hacks we could use to workaround this, but I'm not sure they are worth it.

xref: conda/conda-build#2360

Added PR ( dfornika#2 ) to drop this. Should fix the build issue.

This does not work correctly as `conda-build` strips out the `.git` directory from `METADATA`. There are no clean fixes for this issue. As we already try to initialize the package directory when Julia starts, this will be handled anyways on the user's machine. In the end, this may be best to avoid having a potentially out-of-date index in the package.

jakirkham · 2017-09-12T19:59:50Z

cc @tkelman

tkelman · 2017-09-12T21:03:12Z

recipe/build.sh

-chmod +x "$PREFIX/bin/julia"
-
+# Configure juliarc to use conda environment
+cp -f "${RECIPE_DIR}/juliarc.jl" "${PREFIX}/etc/julia/juliarc.jl"


should append, not replace

It looks like an empty file with a comment about how we can add stuff to it. Why not just replace it?

it isn't empty on all platforms

Ah. So what sort of stuff ends up there then?

tkelman · 2017-09-12T21:08:01Z

recipe/juliarc.jl

+
+if !("JULIA_PKGDIR" in keys(ENV))
+    ENV["JULIA_PKGDIR"] = joinpath(JULIA_PREFIX, "share", "julia", "site")
+    Pkg.init()


this probably shouldn't be done on every startup, only if it hasn't been initialized yet

What's the preferred way to check if the package repository has been initialized?

Was thinking about that. It seems to only take a toll the first time (as it is initializing everything from scratch). What sort of penalties would we be taking on for subsequent calls?

Initially we tried to run this once during the build, but there was a conda-build issue that blocked it. ( conda/conda-build#2360 ) In the end, not sure we should be running this at build time anyways as it would go stale and make the package larger.

Could add a post-link step to handle this instead. That would run this step once only when julia gets installed. Seems like the best of both worlds really.

OTOH we could just never initialize Pkg at all. Would make it slow for the user the first time they run something with Pkg, but that would be the case anyways, right?

FWIW borrowed this strategy from SO answer. Related to this, is it necessary to call Pkg.__init__() or is that already handled by Pkg.init()?

jakirkham · 2017-09-12T21:45:49Z

recipe/build.sh

-chmod +x "$PREFIX/bin/julia"
-
+# Configure juliarc to use conda environment
+"${PREFIX}/etc/julia/juliarc.jl" << "${RECIPE_DIR}/juliarc.jl"


I think you want cat "${RECIPE_DIR}/juliarc.jl" >> "${PREFIX}/etc/julia/juliarc.jl".

Oops. Was trying to avoid a 'useless use of cat award.'

jakirkham · 2017-09-12T21:59:14Z

recipe/juliarc.jl

+if !("JULIA_PKGDIR" in keys(ENV))
+    ENV["JULIA_PKGDIR"] = joinpath(JULIA_PREFIX, "share", "julia", "site")
+    Pkg.init()
+    Pkg.__init__()


Let's go ahead and drop this and the line above it (i.e. Pkg.init()).

jakirkham · 2017-09-12T22:18:05Z

recipe/juliarc.jl

+
+if !("JULIA_PKGDIR" in keys(ENV))
+    ENV["JULIA_PKGDIR"] = joinpath(JULIA_PREFIX, "share", "julia", "site")
+    pop!(Base.LOAD_CACHE_PATH)


Need to change this line to Base.LOAD_CACHE_PATH[1] = joinpath(ENV["JULIA_PKGDIR"], "lib", string("v", join(split(string(VERSION), ".")[1:2], "."))) now that we have removed the Pkg.init() stuff.

jakirkham · 2017-09-12T22:33:37Z

recipe/juliarc.jl

+
+if !("JULIA_PKGDIR" in keys(ENV))
+    ENV["JULIA_PKGDIR"] = joinpath(JULIA_PREFIX, "share", "julia", "site")
+    Base.LOAD_CACHE_PATH[1] = ENV["JULIA_PKGDIR"]


Sorry I think my edits to that comment didn't show up.

This needs to be Base.LOAD_CACHE_PATH[1] = joinpath(ENV["JULIA_PKGDIR"], "lib", string("v", join(split(string(VERSION), ".")[1:2], "."))).

This makes it $PREFIX/share/julia/site/lib/v0.5 for 0.5.x.

jakirkham · 2017-09-13T00:58:11Z

LGTM. This is consistent with its behavior from before and removes the wrapper script (the intended goal). While there may be other discussions/changes that need to occur, would recommend we save them for new issues/PRs.

jakirkham · 2017-09-13T01:20:53Z

Thanks @dfornika. Also thanks @tkelman for the review.

* Removed wrapper script * Bumped build number * Use juliarc (conda-forge#1) * Add a custom juliarc.jl Needed to ensure packages and other relevant information to the Julia install are tracked in the conda environment. * Install custom juliarc Make sure that that our custom juliarc is available in the standard location. * Create the package directory Make sure that Julia initializes our package directory and includes the standard packages in it. * Bump build number to 1 Needed to repackage after removal of the wrapper script, inclusion of juliarc, and inclusion of the prepopulated package directory. * Removed JULIA_HISTORY setting * Drop package initialization (conda-forge#2) This does not work correctly as `conda-build` strips out the `.git` directory from `METADATA`. There are no clean fixes for this issue. As we already try to initialize the package directory when Julia starts, this will be handled anyways on the user's machine. In the end, this may be best to avoid having a potentially out-of-date index in the package. * Append juliarc.jl instead of replace * Append juliarc.jl the right way * Added post-link script for Pkg.init() * Removed Pkg.init() from juliarc.jl * Correct setting Base.LOAD_CACHE_PATH * Correct setting Base.LOAD_CACHE_PATH again (cherry picked from commit 19d96fa)

* Removed wrapper script * Bumped build number * Use juliarc ( dfornika#1 ) * Add a custom juliarc.jl Needed to ensure packages and other relevant information to the Julia install are tracked in the conda environment. * Install custom juliarc Make sure that that our custom juliarc is available in the standard location. * Create the package directory Make sure that Julia initializes our package directory and includes the standard packages in it. * Bump build number to 1 Needed to repackage after removal of the wrapper script, inclusion of juliarc, and inclusion of the prepopulated package directory. * Removed JULIA_HISTORY setting * Drop package initialization ( dfornika#2 ) This does not work correctly as `conda-build` strips out the `.git` directory from `METADATA`. There are no clean fixes for this issue. As we already try to initialize the package directory when Julia starts, this will be handled anyways on the user's machine. In the end, this may be best to avoid having a potentially out-of-date index in the package. * Append juliarc.jl instead of replace * Append juliarc.jl the right way * Added post-link script for Pkg.init() * Removed Pkg.init() from juliarc.jl * Correct setting Base.LOAD_CACHE_PATH * Correct setting Base.LOAD_CACHE_PATH again (cherry picked from commit 19d96fa)

Removed wrapper script

c1ad13b

Bumped build number

0869a65

jakirkham mentioned this pull request Sep 12, 2017

Use juliarc dfornika/julia-feedstock#1

Merged