-
Notifications
You must be signed in to change notification settings - Fork 161
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 command line option --bare to start GAP without even needed packages (developer tool) #2952
Conversation
While it's yet another .travis.yml line, do we want to test GAP works with |
Not all tests work (but a suprising number does!). I'll look at adding a "testbare" target. |
What is the difference between --bare and -N (for noautoload)? |
@hulpke |
If you will look at "testbare" target, you may also start GAP there with |
f1de3c2
to
c782de8
Compare
Codecov Report
@@ Coverage Diff @@
## master #2952 +/- ##
==========================================
+ Coverage 84.55% 84.76% +0.21%
==========================================
Files 698 687 -11
Lines 345303 335870 -9433
==========================================
- Hits 291963 284700 -7263
+ Misses 53340 51170 -2170
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seem fine to me (I also rebased this to see if it still builds fine, and that revealed that the first of it was already merged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be useful to use --bare
in one of the tests or in a separate Travis test, to ensure that we exercise it and that GAP still starts with it. This will also allow to start GAP with -O
to disable loading obsoletes to prevent them crawling into the GAP library again.
What is the reasoning for this? If a package is "needed" GAP is allowed to fail arbitrarily badly without it, including not loading the library. If you need to start GAP without GAPDoc for some reason, then maybe GAPDoc is not needed, just recommended for interactive sessions or something? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this PR and think that it should be merged as-is.
I'll note that there are currently four needed packages: GAPDoc, as well as primgrp, smallgrp, and transgrp.
From my point of view, the purpose of --bare
is to easily let one play around and see what happens without some or all of these packages loaded. That will help us (in time) work out the answer to @stevelinton's question of whether we actually need each of the required packages. It stops us having to hack around too much to explore these questions.
I don't think that we need a Travis test for the --bare
option. It's not a user-facing feature, and as @stevelinton mentioned above, GAP is allowed to fail arbitrarily badly without its required packages. So I'm not sure what we'd be testing: if using --bare
suddenly started to fail loading GAP properly, I don't think that's a reason for our Travis tests to fail. That would implicitly require any PR authors to take care that they don't break --bare
, even though we shouldn't guarantee that --bare
works. That doesn't seem fair.
In any case, @markuspf seems to be no longer getting involved in GAP development, so if people want to require changes to this then we'll have to do it ourselves. If that's not forthcoming, then we either have to merge as-is, or close.
(I just rebased this on today's master
branch, and --bare
loads GAP fine.)
I'm fine with this, but as @wilfwilson says, it should not be user-facing, and for the same reason should not be part of the tests that get run automatically on Pull Requests unless we want to change the rules on what it means for a package to be needed. This is a specialist feature for debugging aspects of library and package loading. |
@stevelinton would you feel if the text describing the
Or something of that kind? |
Something of the kind.
|
Allow starting GAP without any packages
c782de8
to
3720c08
Compare
I prefer that, I've made the change (and rebased). |
@wilfwilson If this is not user-facing are release notes still needed? |
@wucas I think so. I would recommend using the title of this PR (which I just updated) as the text for the release notes. |
This requries (and includes) #2950.