#40516 closed enhancement (fixed)
Two new variants + text editor modeline
Reported by: | jpolmsted@… | Owned by: | kjellpk (Kjell Konis) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | haspatch | Cc: | larryv (Lawrence Velázquez), cooljeanius (Eric Gallager) |
Port: | R |
Description
I added two new variants to the current R Portfile.
check: runs the "check-all" tests for the R build during the Test Macports phase.
tests: installs testing code and output for subsequent testing of the installation during the destroot Macports phase.
Also, per the documentation, I added a modeline up top.
Attachments (1)
Change History (13)
comment:1 Changed 11 years ago by mf2k (Frank Schima)
Keywords: | haspatch added |
---|---|
Owner: | changed from macports-tickets@… to kjell.konis@… |
Version: | 2.2.0 |
comment:2 Changed 11 years ago by larryv (Lawrence Velázquez)
Comments:
- Setting
test.run yes
does not actually run any tests; it just enables running them viaport test R
. So thetest.*
options should be at the root level of the Portfile, and there should not be a variant. (Otherwise you’d have to runport test R +check
.) - Would it be detrimental to just run
make install-tests
always, instead of inside a variant?
comment:3 follow-up: 4 Changed 11 years ago by jpolmsted@…
I'm confused about the opposition. This is the behavior intended by me and seems completely consistent with the test phase. Am I missing something? Under this setup, checking is optional (as it is with a manual install) and it happens after the build and before the install.
From an R user's perspective, I don't think there is a downside. It just copies files to the installation prefix that already exist and can be used.
comment:4 follow-ups: 5 11 Changed 11 years ago by larryv (Lawrence Velázquez)
Replying to jpolmsted@…:
I'm confused about the opposition. This is the behavior intended by me and seems completely consistent with the test phase. Am I missing something? Under this setup, checking is optional (as it is with a manual install) and it happens after the build and before the install.
This is not true; the test phase is not run during a port installation, even if the Portfile sets test.run yes
. Users would already have to explicitly run the tests with port test R
, so they should not have to select a variant on top of that.
From an R user's perspective, I don't think there is a downside. It just copies files to the installation prefix that already exist and can be used.
Hm. If installing the tests isn’t terribly inconvenient (build time, disk space, etc.), then they should probably always be installed.
comment:5 Changed 11 years ago by jpolmsted@…
Replying to larryv@…:
Replying to jpolmsted@…:
I'm confused about the opposition. This is the behavior intended by me and seems completely consistent with the test phase. Am I missing something? Under this setup, checking is optional (as it is with a manual install) and it happens after the build and before the install.
This is not true; the test phase is not run during a port installation, even if the Portfile sets
test.run yes
. Users would already have to explicitly run the tests withport test R
, so they should not have to select a variant on top of that.
Thanks, that makes sense. I'll update the patch tomorrow to be consistent with port test R
usage.
From an R user's perspective, I don't think there is a downside. It just copies files to the installation prefix that already exist and can be used.
Hm. If installing the tests isn’t terribly inconvenient (build time, disk space, etc.), then they should probably always be installed.
I don't disagree, but I'll err on the side of the R developers here. They didn't (and still don't) choose to include them. But the port maintainer can easily change it if wanted.
Changed 11 years ago by jpolmsted@…
Attachment: | Portfile-R.diff added |
---|
comment:6 Changed 11 years ago by jpolmsted@…
I've updated the patch. For the current version:
- Test phase-specific changes are root level.
- Installation of tests for later use still occurs only by requesting the "tests" variant.
- Text editor modeline added to the Portfile.
comment:7 Changed 11 years ago by kjellpk (Kjell Konis)
Maintainer here. Looks good to me. Thanks for the patch. The next update to R is scheduled for the 25th so I'll make these changes with the version bump next week.
comment:9 Changed 11 years ago by kjellpk (Kjell Konis)
Maintainer here. The R version bump in #40602
(1) adds the text editor modeline
(2) enables testing (i.e., sets test.run to yes). I set test.target to check rather than check-all because check-all requires that additional CRAN packages be installed in order complete cleanly
(3) adds a test variant that installs the tests for later use
comment:10 Changed 11 years ago by larryv (Lawrence Velázquez)
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:11 Changed 11 years ago by cooljeanius (Eric Gallager)
Replying to larryv@…:
Replying to jpolmsted@…:
I'm confused about the opposition. This is the behavior intended by me and seems completely consistent with the test phase. Am I missing something? Under this setup, checking is optional (as it is with a manual install) and it happens after the build and before the install.
This is not true; the test phase is not run during a port installation, even if the Portfile sets
test.run yes
. Users would already have to explicitly run the tests withport test R
, so they should not have to select a variant on top of that.
I've always found this counterintuitive. There should be some sort of option to automatically run the test phase during a normal port installation...
Thanks. In the future, please Cc the port maintainers (
port info --maintainers R
).