Skip to content

Conversation

@lgoettgens
Copy link

This adresses oscar-system#5345 (comment) in the way how we handle installation of deposited GAP packages inside of GAP.jl.

Additionally, this addresses a part of the issues mentioned in the CI logs of oscar-system#5345, namely (https://github.com/oscar-system/Oscar.jl/actions/runs/19213809957/job/54920128504?pr=5345#step:10:340)

      From worker 4:	#I  Package already installed at target location
      From worker 4:	#I  Target directory /home/oscarci-tester/ssd-data/ssd-runner-04/julia/scratchspaces/c863536a-3901-11e9-33e7-d5cd0df7b904/gap_packagedir_v4.15/pkg/Origami-2.0.1 exists and is non-empty
      From worker 4:	#I  Appending '.old' to old version directory
      From worker 4:	#I  Possible error detected, see log:
      From worker 4:	#I    /usr/bin/mv: cannot move '/home/oscarci-tester/ssd-data/ssd-runner-04/julia/scratchspaces/c863536a-3901-11e9-33e7-d5cd0df7b904/gap_packagedir_v4.15/pkg/Origami-2.0.1' to '/home/oscarci-tester/ssd-data/ssd-runner-04/julia/scratchspaces/c863536a-3901-11e9-33e7-d5cd0df7b904/gap_packagedir_v4.15/pkg/Origami-2.0.1.old/Origami-2.0.1': Directory not empty
      From worker 4:	#I  Could not rename old package directory

Note: there are more issues mentioned in this log, I commented about them at oscar-system#5345 (review).

@Sebas777-gif please wait with merging this until @fingolfin and/or @ThomasBreuer had a chance to look at this.

abspath(artifact"GAP_pkg_origami")
]; recursive = true))
GAP.Packages.load("Origami")
end

Choose a reason for hiding this comment

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

Why can't we call GAP.Globals.ExtendPackageDirectories already in an __init__ function here?
For that, we need just GAP.jl, not Oscar.jl.

(Conversely, GAP.Packages.load("Origami") could then be called in src/Oscar.jl where also the other GAP packages get loaded. This would happen in particular before the __GAP_info_messages_off() call which then affects also an InfoClass that is perhaps introduced in the Origami package.)

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, this is a lot cleaner with your suggested change. I think this was just not possible with some intermediate version I was tinkering with, and I forgot to check again in the end.

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