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

SystemRequirements: ImageMagick++? #28

Closed
HenrikBengtsson opened this issue Aug 30, 2018 · 6 comments · Fixed by #29
Closed

SystemRequirements: ImageMagick++? #28

HenrikBengtsson opened this issue Aug 30, 2018 · 6 comments · Fixed by #29

Comments

@HenrikBengtsson
Copy link
Contributor

Is the following really required:

SystemRequirements: ImageMagick++: ImageMagick-c++-devel (rpm) or libmagick++-dev (deb)

or is it related to {magick} being Suggests:ed?

Background: Having experienced enough out-dated academic compute environments in my life, I find it unfortunate if {princurve} gains required dependencies that are not relevant to the core business and that may complicate and even prevent installation for end users. For instance, it can take days, weeks, months, to convince sysadms to install a missing system library.

@rcannood
Copy link
Owner

Hello Henrik,

It is indeed only Suggested. Would you recommend the system requirements to be displayed differently? I could also just remove the vignettes altogether.

Robrecht

@gavinsimpson
Copy link

Then I don't think princurve should have this stated. Writing R Extensions (WRE) says

Dependencies external to the R system should be listed in the ‘SystemRequirements’ field...

As such the libraries currently listed in System Requirements are not dependencies of princurve but of the magick package, which presumably has the appropriate SystemRequirements?

You shouldn't need to do anything other than list magick in Suggests to use it in the Vignette. (You may want to check what WRE says about using Suggests packages conditionally in examples, tests, and vignettes.)

@HenrikBengtsson
Copy link
Contributor Author

I second what @gavinsimpson says. Let {magick} take care of that. It would also become infeasible if package maintainers would have to keep track, maintain, and list all SystemDependencies for all of the dependencies (also recursively). It is also very likely that such a model would get outdated/broken quite quickly.

@rcannood
Copy link
Owner

rcannood commented Sep 2, 2018

@HenrikBengtsson I removed any exotic packages from the DESCRIPTION to ensure backward compatibility. Is the current state of the @devel branch ok for you?

@HenrikBengtsson
Copy link
Contributor Author

HenrikBengtsson commented Sep 2, 2018

I think you only need to remove the SystemRequirements. It's ok to keep Suggests:ed package.
I also think it is ok to use those in the vignette.

Note that vignettes are pre-built during R CMD build (vignettes/ is processed and docs/ is produced). Vignettes are not built during R CMD install, which means it is ok to keep dependencies on the {magick} package etc in there. A user without {magick}, will still be able to install {princurve}, i.e. Suggests: dependencies are not a show stopper.

@rcannood
Copy link
Owner

rcannood commented Sep 5, 2018

I was unsatisfied with the vignette, which is why I decided to remove it for now. I'll create a new one when I have the time for it. I'll be merging the devel branch now :)

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 a pull request may close this issue.

3 participants