opam icon indicating copy to clipboard operation
opam copied to clipboard

Sandbox: advise user to launch reinit if updated in opam side

Open MSoegtropIMC opened this issue 5 years ago • 10 comments

As far as I can tell the macOS sandbox script (https://github.com/ocaml/opam/blob/master/src/state/shellscripts/sandbox_exec.sh) is copied to .opam/opam-init/hooks during opam init. If opam is updated, this file is not touched. This has the effect that people have recent versions of opam with a not working sandbox. This might depend on the way how opam is installed and/or updated.

A notable effect of this is that any project build with remake does not build in case the sandbox file is too old. I meanwhile check besides the version of opam some properties of the sandbox script in my meta install scripts.

MSoegtropIMC avatar Sep 15 '20 13:09 MSoegtropIMC

In order to update sandbox scripts, a reinit is needed : opam init --reinit -ni. It is advised in all release blog posts (at the bottom).

Is the problem occurs even with a reinit?

rjbou avatar Sep 15 '20 14:09 rjbou

Is the problem occurs even with a reinit?

I can't tell immediately - it was an issue reported by a user of my opam based Coq meta build scripts. I will contact the user and try to clarify this.

Is there a way to check which version of opam created or last updated the .opam folder, so that I could check this in my scripts and automatically suggest and run the opam reinit?

I can definitely check if the sandbox script is suitable and suggest the reinit in case it is not.

MSoegtropIMC avatar Sep 15 '20 17:09 MSoegtropIMC

It is possible if there is changes at opam root (cf. new field opam-version at the top of the .opam/config), but it won't help you about sandbox scripts. To be sure, by default at each new version of opam, advise to run the reinit command. It also can be run outside any opam new release, to be sure to regenerate scripts. You can also see opam init options to customize the reinit.

rjbou avatar Sep 16 '20 13:09 rjbou

Note that there is a fix of -ni option in 2.1.0~alpha.

rjbou avatar Sep 16 '20 13:09 rjbou

Something that may help you for integration: in 2.1.0~alpha, we added a hash of sandbox script, to avoid overwriting user's modifications (cf. #4020). That hash (md5) is stored in .opam/opam-init/hooks/sandbox.sh.hash. You can check that hash with the opam repo one at given version (e.g. for 2.1.0~beta on linux https://github.com/ocaml/opam/blob/2.1.0-beta/src/state/shellscripts/bwrap.sh), and advice a reinit if it differ from the opam version you need. Keep in mind that hashes can differ because of local user modification.

rjbou avatar Sep 16 '20 14:09 rjbou

Thanks for all the hints! I am currently still on opam 2.0.7 (I am a bit limited here since I need Windows support). The way I will proceed for now is that I detect with grep if the sandbox file has certain features I need and if not suggest the opam reinit. I think this situation is anyway quite rare and this method should fix at least 99% of the rare cases.

I think long term it would be best if opam would know on its own of the .opam folder is up to date or needs updating and suggest proper action on common opam commands.

MSoegtropIMC avatar Sep 16 '20 14:09 MSoegtropIMC

I can confirm that opam init --reinit --no-setup fixes the script (I run it without -i from my script).

So please feel free to close this issue. I still would recommend to have opam check and report this automatically.

MSoegtropIMC avatar Sep 16 '20 19:09 MSoegtropIMC

Good to hear!

Opam already have a detection of its internal format changes, but it doesn't applies to sandbox scripts. Another mechanism should be put in place for that, to suggest a reinit in case of obsolete sandbox script.

rjbou avatar Sep 17 '20 15:09 rjbou

Another mechanism should be put in place for that, to suggest a reinit in case of obsolete sandbox script.

Will you create an issue for this or should I or do we keep this issue for this?

MSoegtropIMC avatar Sep 17 '20 17:09 MSoegtropIMC

we can keep this one, and changing it's name

rjbou avatar Sep 22 '20 07:09 rjbou