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

reorganize & cleanup #157

Merged
merged 39 commits into from
Dec 29, 2021
Merged

reorganize & cleanup #157

merged 39 commits into from
Dec 29, 2021

Conversation

ngam
Copy link
Contributor

@ngam ngam commented Dec 24, 2021

Closes #67
Closes #14
Closes #140
Closes #87
Closes #160
Closes #138
Closes #76

TODO:

  • isolate julia inside conda
  • sync julia project with conda env
  • stack julia envs

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

mjohnson541 and others added 6 commits August 10, 2021 08:29
* This uses the activate.sh and deactivate.sh scripts that backup the environment variable.
* Create a new shared environment located in the site depot for the environment rather than continuing to use the default one in ~/.julia/environments/v#.#.
@conda-forge-linter
Copy link

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.

@ngam
Copy link
Contributor Author

ngam commented Dec 24, 2021

@mkitti about the tests, should we try to expand them?

# all except stdlib, add "opaque_closure", "filesystem" with 1.7
# [not osx] due to failure on math.jl:296, pre-haswell processor

@ngam
Copy link
Contributor Author

ngam commented Dec 25, 2021

@mkitti feel free to review and merge. Or let me know if there are other cleanup/organization things we should deal with first. We should also move this to the other branch (v1.6.5) once we settle on a procedure.

Should we expand the tests above?

@ngam
Copy link
Contributor Author

ngam commented Dec 25, 2021

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/julia-feedstock/actions/runs/1622318079.

@ngam
Copy link
Contributor Author

ngam commented Dec 25, 2021

These pass just fine locally: # all except stdlib, add "opaque_closure", "filesystem" with 1.7

@ngam
Copy link
Contributor Author

ngam commented Dec 25, 2021

Okay, all look good --- will test the stdlib as well soon

@mkitti
Copy link
Contributor

mkitti commented Dec 28, 2021

1.7.2 is probably weeks to a few months away. 1.7.0 and 1.7.1 were separated by three weeks.

1.8.0 is likely 6 months to a year away.

Copy link
Contributor

@mkitti mkitti left a comment

Choose a reason for hiding this comment

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

Why are we keeping the old juliarc.jl and pre-unlink.sh around?

Is there any functionality in pre-unlink.sh that should be replicate?

recipe/build.sh Outdated Show resolved Hide resolved
@mkitti
Copy link
Contributor

mkitti commented Dec 29, 2021

I'm good to merge this if you are, @ngam .

@mkitti mkitti mentioned this pull request Dec 29, 2021
5 tasks
@ngam
Copy link
Contributor Author

ngam commented Dec 29, 2021

Should we address #161 or wait?

@ngam
Copy link
Contributor Author

ngam commented Dec 29, 2021

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/julia-feedstock/actions/runs/1635585005.

@ngam ngam merged commit bd5133e into conda-forge:master Dec 29, 2021
@zklaus
Copy link

zklaus commented Jan 2, 2022

It's great to see the rapid improvements coming in here! Big steps forward on many fronts.

But if you want a bit more community engagement you might want to consider slowing down just a little bit. This PR, which closes 7 issues, some of which have been around for a long time and have been opened and worked upon by other people, took all of 5 days from creation to merging. Over the Christmas days.
Still, I don't want to dampen the enthusiasm; it really is great to see this move forward.

For now, I have two concerns/comments:

The issue of separate depots for separate environments is really all about binary compatibility. Basically, anything that has been compiled for one environment cannot reliably be used with another environment because linked libraries may have changed in an ABI incompatible way (cf the recent libunwind problems, but I would expect this with a number of libraries, e.g. for I/O of special file formats like netCDF) and the compilers themselves may be different. If compiled bits are shared, this will lead to surprising segfaults in the future. This kind of problem is not likely to show up in tests that use similar environments created at roughly the same time, but is virtually guaranteed if very different environments with library overlap are used, particularly when those environments are created a long time apart.
To me, it is not clear to what degree compiled bits are an integral part of the Julia depots, but this is my main problem with ~/.julia currently (i.e. conda-forge Julia <=1.6).

My second concern is that it is not completely clear to me to what degree this PR really addresses the issues it closes. For example, #14 is about packaging Julia packages for conda-forge, i.e. how should I create the conda-forge feedstock julia-rainfarm from https://github.com/jhardenberg/RainFARM.jl. While sorting out the depot paths certainly helps and might be necessary, from reading this PR I still don't know how to do this or where to look up the documentation. In terms of documentation, at a minimum I would expect a bullet point in the enumeration for new packages.
This is probably beyond the scope of this PR, but in that case we might want to keep #14 open for now.

@ngam
Copy link
Contributor Author

ngam commented Jan 2, 2022

But if you want a bit more community engagement you might want to consider slowing down just a little bit. This PR, which closes 7 issues, some of which have been around for a long time and have been opened and worked upon by other people, took all of 5 days from creation to merging. Over the Christmas days.

Yeah, sorry... But, I'd rather interested people (like yourself) come back and reopen the issues and explain their current needs/expectations. It's very difficult for us to guess what people were thinking about 4+ years ago especially as the ecosystems (julia as well as conda) have moved so much since then. Obviously, we couldn't address all concerns (and you bring up really important ones) --- to help us push forward, could you please reopen issues/PRs that you think weren't adequately addressed and add your perspective?

I will start by opening two issues based on your two main concerns and we will work --- together and less rapidly --- to address them.

To me, it is not clear to what degree compiled bits are an integral part of the Julia depots, but this is my main problem with ~/.julia currently (i.e. conda-forge Julia <=1.6).

For this, @mkitti was of the opinion that we shouldn't change anything in the 1.6.x and just bring all newer changes to 1.7.1+. I concurred with his view (we don't want to mess up the setups of people who got comfortable with those versions...)

@ngam
Copy link
Contributor Author

ngam commented Jan 2, 2022

My second concern is that it is not completely clear to me to what degree this PR really addresses the issues it closes. For example, #14 is about packaging Julia packages for conda-forge, i.e. how should I create the conda-forge feedstock julia-rainfarm from https://github.com/jhardenberg/RainFARM.jl

I certainly didn't read it this way. I am not sure if this is even applicable now that julia has their own ecosystem and package-building setup. Our real goal here was to better isolate julia inside conda envs, so that a user can have many different julias (via conda) and also a separate standalone julia smoothly. How the user ends up using such isolation is obviously none of our business, right? And I am not sure if we should pursue changes to actually enmesh julia and conda more tightly; again, my thinking/perspective was to better isolate them from each other (not vice versa). But that's just one single opinion and I am happy to discuss this further and work on bringing changes/improvements you'd like to see :)

@ngam
Copy link
Contributor Author

ngam commented Jan 2, 2022

7 issues

only 3. The others are PRs.

@mkitti
Copy link
Contributor

mkitti commented Jan 2, 2022

Native machine code is not cached by Julia during normal operation, so I do not expect there to be be ABI issues for compiled code generated from Julia unless one is making direct accalls. What is cached is lowered code and type inference on a per Julia environment basis in $JULIA_DEPOT/compiled. That said this does change between Julia versions and is under active development.

Thr chosen defaults for the 1.7 releases are quite conservative with unique Julia depots and environments for each conda environment. My concern is actually that is too conservative and that we are duplicating too many things.

The use of PackageCompiler.jl, which actually involves native code generation and storage is another matter, but its location of the generated system image shared library is determined by the user.

@zklaus
Copy link

zklaus commented Jan 2, 2022

Thanks for your replies, @ngam and @mkitti. I'll pick up the discussion in #14, #164 and possibly new issues as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants