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

Support etc_root and etcexec_root in opam-installer #3958

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hongchangwu
Copy link

Use case

Startup scripts for Linux system daemons are traditionally installed under /etc/init.d.

doc/pages/Manual.md Outdated Show resolved Hide resolved
@AltGr
Copy link
Member

AltGr commented Aug 29, 2019

Not really sure where this would be useful: it would basically allow to write to /usr/etc (or ~/.opam/some-switch/etc, even) rather than /etc ?

@hongchangwu
Copy link
Author

hongchangwu commented Aug 30, 2019

It's the same contrast between share and share_root.

With the current etc variable files can be only installed under e.g. /usr/etc/<package name>, but not under /usr/etc/ or /usr/etc/init.d.

@rjbou
Copy link
Collaborator

rjbou commented Aug 30, 2019

What is you use case ? By default, opam won't install to root directories, but on its the switch prefix (cf. etc_dir definition)

@dra27
Copy link
Member

dra27 commented Aug 30, 2019

Note that PR’s concern seems to be about opam-installer rather than opam

@hongchangwu
Copy link
Author

hongchangwu commented Aug 30, 2019

Hi, the purpose of this PR is not to install to root directories, but rather to be able install things under <prefix>/etc/ directory (etc install things under <prefix>/etc/<package name> )

The use case is stated in the PR description: Linux system daemon scripts are traditionally installed under <prefix>/etc/init.d. Currently AFAIK there is no way to do that with opam-installer. Even hacks such as using ../ in the path does not work because anything installed under etc will not be executable, and the scripts under <prefix>/etc/init.d need to be executable.

BTW this PR (#1141) introduced share_root to address a similar issue.

@hongchangwu
Copy link
Author

hongchangwu commented Aug 30, 2019

@dra27 right, opam doesn't install to system directories, but it's perfectly conceivable for system package managers such as debian to use opam-installer when packaging OCaml things. As such it would be good if opam-installer is able to support install system startup scripts under <prefix>/etc/init.d.

Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. Just to minimal changes to do, and it's ok to merge.

doc/pages/Manual.md Outdated Show resolved Hide resolved
src/format/opamFile.ml Show resolved Hide resolved
@hongchangwu
Copy link
Author

@rjbou thanks for the reply. I'll take a look at this soon.

@rjbou
Copy link
Collaborator

rjbou commented Jan 7, 2020

ping @hongchangwu

@AltGr AltGr added this to the Next milestone Mar 9, 2020
@rjbou rjbou removed this from the Next milestone May 20, 2020
@rgrinberg
Copy link
Member

A year later, ping @hongchangwu :)

@toots
Copy link

toots commented Jan 3, 2021

Hi there! Would love to see this merged as well. We could definitely use it for liquidsoap. There's a corresponding PR for dune here: ocaml/dune#4077

@rjbou
Copy link
Collaborator

rjbou commented Jan 6, 2021

updated

@dra27
Copy link
Member

dra27 commented Mar 17, 2021

At this stage, this obviously isn't going to be in 2.1, but there are a couple of things which need to be considered. It should be an error to have these fields in a file headed "opam-version: 2.0" and we should also ensure that when writing these files opam only uses opam-version: "2.1" if there are etc_root and etcexec_root fields to write.

@hongchangwu
Copy link
Author

hongchangwu commented Apr 26, 2021

Apologies for not responding earlier, this has fallen off my radar. Yeah I think it will be nice to support this. I can update the PR.

@hongchangwu
Copy link
Author

Rebased.

@dra27 dra27 added this to the 2.2.0~alpha milestone Apr 27, 2021
@dra27
Copy link
Member

dra27 commented Apr 27, 2021

Thanks, @hongchangwu! We still need to address what to do about opam-version and ensuring that these fields are an error if opam-version: "2.0" was encountered. We're just in the process of getting opam 2.1 to release candidate, but I've put this PR in the 2.2.0~alpha milestone to ensure it gets remembered.

@dra27 dra27 added this to PR in progress in Opam 2.2.0 via automation Jun 25, 2021
@dra27
Copy link
Member

dra27 commented Jun 25, 2021

This PR is definitely still remembered! The machinery for validating fields by version (i.e. rejecting etc_root in foo.install if it has opam-version: "2.0") is likely to be added as part of the opam file rewriting plugin, and once that's in it would unblock the ability to merge this.

@dra27 dra27 self-assigned this Jun 25, 2021
@rjbou rjbou added PR: QUEUED Pending pull request, waiting for other work to be merged or closed and removed PR: NEEDS UPDATE labels Sep 1, 2021
@dra27 dra27 added this to PR in Progress in Opam 2.3 via automation Feb 11, 2022
@dra27 dra27 removed this from PR in progress in Opam 2.2.0 Feb 11, 2022
@dra27 dra27 removed this from the 2.2.0~alpha milestone Feb 11, 2022
@dra27
Copy link
Member

dra27 commented Feb 11, 2022

We're not looking at file format changes in 2.2, so bumping this for consideration in 2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: QUEUED Pending pull request, waiting for other work to be merged or closed
Projects
Opam 2.3
  
PR in Progress
Development

Successfully merging this pull request may close these issues.

None yet

6 participants