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

Add DirectoriesPackageProgramsOverrides #690

Conversation

fingolfin
Copy link
Member

For use in Julia wrapper packages for GAP packages that included a
kernel extension or otherwise compiled code: they carry the (GAP) source
code for a GAP package in one Julia artifact, and the compiled binary in
another (via a JLL). It is easy enough to point GAP at the former, via
SetPackagePath; but we need this patch to also handle the latter.

For use in Julia wrapper packages for GAP packages that included a
kernel extension or otherwise compiled code: they carry the (GAP) source
code for a GAP package in one Julia artifact, and the compiled binary in
another (via a JLL). It is easy enough to point GAP at the former, via
`SetPackagePath`; but we need this patch to also handle the latter.
@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

Merging #690 (ab02115) into master (53fadd2) will increase coverage by 0.02%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #690      +/-   ##
==========================================
+ Coverage   82.86%   82.89%   +0.02%     
==========================================
  Files          45       46       +1     
  Lines        3595     3607      +12     
==========================================
+ Hits         2979     2990      +11     
- Misses        616      617       +1     
Impacted Files Coverage Δ
lib/pkg.g 90.90% <90.90%> (ø)
src/GAP.jl 82.79% <100.00%> (+0.18%) ⬆️

@ThomasBreuer
Copy link
Member

This is fine as a temporary change, but I think that eventually the desired behaviour should be achieved by a documented extension on the GAP side.
(Currently GAP claims (not in the documentation but via the code) that everything that is used from a loaded package lies in one directory. Now GAP.jl silently changes this, and relies on the fact that nobody else has a similar idea.)

@fingolfin
Copy link
Member Author

I am not sure I understand the hypothetical problem with "someone else having a similar idea". Who should that be? Package authors? I don't think we'd accept such a package into the distribution. Other people packaging GAP? But that would not in any way interact with the GAP we package.

But yes, such a change is needed on the GAP side eventually, too, simply because we want to eventually support something like make install for GAP and for packages, and that doesn't work with the approach of "putting everything into one directory" (unless one counts /usr.

However, doing that "properly" is non-trivial, and in the past several years, I have seen zero effort by anybody to work on that, and we need something that works now. I am happy to discuss a "better" solution eventually, but for now, this does the trick, and I see no real downsides, as long as it is 100% under our control...

@fingolfin fingolfin merged commit 8ef1543 into oscar-system:master Sep 9, 2021
@fingolfin fingolfin deleted the mh/DirectoriesPackageProgramsOverrides branch September 9, 2021 21:21
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

Successfully merging this pull request may close these issues.

2 participants