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

2.0.2: backported commits #3669

Merged
merged 28 commits into from Dec 10, 2018

Conversation

Projects
None yet
7 participants
@rjbou
Copy link
Collaborator

rjbou commented Nov 8, 2018

Backported commit for 2.0.2 release.

commit issue descr
4d39333 system: suffix .out for read_command_output stdout files
38dd76d #3622 state config: resolve default opam root path
4eb7be9 filename: add concat and resolve operator
8aff7ad Fix some cases of listing coinstallable packages
4952b36 pin: add aux files on pinning 1.2 opam files
60e5cfb option: deprecated no-aspcud
1f19a2b #3654 locked: check consistency with opam file when reading lock file to suggest regeneration message
7e44f01 #3508 pin: upgrade pin depends on pinning
fe1f88f #3390 option: reinsert deprecated option alias-of & no-autoinstall to display deprecated message
426e0f2 arg: add a deprecated option message display function
25c6267 doc: update man pages
d002196 show: remove pin depends messages
0a6b63e #3662 Fix closure computation in the presence of cycles
471d34c #3660 Handle symlinks in bwrap
62753f8 #3659 Sandbox: allow access to unix sockets
4f3d616 #3652 Do not check hash with --no-checksum on pull_upstream
f85ba9a man: add OPAMNOCHECKSUMS doc
a5cb01c #3660 Sandbox: handle symlinks
52bd2b9 #3652 Do not check hash with --no-checksum on pull_upstream
e9c701c #3607 Sandbox: change one-line conditional to if statement which was incompatible with set -e
016252e Format Upgra#de: add function to upgrade opam file and its auxiliary files
3997d9f #3604 Sandbox: make /var readonly instead of empty and rw
e0826ac #3600 Format upgrade: extract archived source files of version-pinned packages
7d0a673 Core: add is_archive in OpamSystem and OpamFilename
bc86afd #3614 Init: don't fail if empty compiler given
7521abb #3625 Lint: fix light_uninstall flag for error 52
58918d8 Update cold compiler to 4.07.1
@avsm

This comment has been minimized.

Copy link
Member

avsm commented Nov 23, 2018

Lgtm, except a question about upgrades: if the sandbox scripts are updated between opam versions (as they are here), are the hooks in an already-initialised opam installation also updated, or do users need to reinitialise their setup to get the latest versions?

@rjbou

This comment has been minimized.

Copy link
Collaborator

rjbou commented Dec 3, 2018

An action is needed to be performed by users, an opam init --reinit -n . It is usually mentioned in the blog post announcement.

@AltGr

This comment has been minimized.

Copy link
Member

AltGr commented Dec 4, 2018

indeed, our approach — using hooks — makes it extremely customisable, but at the cost of some friendliness for disabling or updates... I already saw confusion on quite a few cases with users expecting opam init --disable-sandboxing to have an effect on an already initialised opam root.

An idea to solve this could be to keep a static definition of all known by-default previous configs (or preferably just their hashes). Then when we detect an opam version bump, we can upgrade automatically states that match the known hashes, and just print a warning when we detect a modified or custom version (a little like Debian does for modified configuration files). This could apply both to the script file and to the hooks configuration, allowing for a --disable option in the latter case (assuming we want to provide such an option :/ )

@AltGr

This comment has been minimized.

Copy link
Member

AltGr commented Dec 4, 2018

LGTM !

rjbou and others added some commits Nov 6, 2018

Fix #3659: Sandboxing on MacOS is too strict (#3663)
This commit allows applications to use unix socket.

It also allows them to write to /dev/dtracehelper. This permission is
not strictly needed, as no program is broken by a denied access. However,
MacOS' dynamic loader (hence almost all the applications) wants to access
it, so the system logs are flooded by messages such as "SandboxViolation:
sh(49331) deny(1) file-write-data /dev/dtracehelper" when using opam.
Handle symlinks in bwrap (#3661)
On some OSes, like CentOS and Arch Linux (so I've read), `/bin` is a symlink to `/usr/bin`, `/lib` is a symlink to `/usr/lib`, etc. 

Mounting them directly with `--ro-bind` resolves the symlinks. This breaks paths like `/bin/../libexec`: when `/bin` is a symlink to `/usr/bin`, `/bin/../libexec` points to `/usr/libexec`; but if it gets resolved first, it points to `/libexec`.

Fixes #3660
Fix closure computation in the presence of cycles (#3670)
When a cycle is present (typically ocaml-base-compiler ⇄ ocaml), relying on the successor to determine if a package should be marked gives an incomplete graph
pin: add aux files on pinning 1.2 opam files (#3687)
to avoid warning 57 (descr error)

@rjbou rjbou force-pushed the rjbou:2.0.2 branch from 52bd2b9 to e1aeeed Dec 6, 2018

@rjbou

This comment has been minimized.

Copy link
Collaborator

rjbou commented Dec 6, 2018

Last updates!

@rjbou rjbou force-pushed the rjbou:2.0.2 branch from e1aeeed to 3d1313e Dec 6, 2018

@avsm

This comment has been minimized.

Copy link
Member

avsm commented Dec 6, 2018

It is usually mentioned in the blog post announcement.

Most users would probably never see this :-( It would be really nice to have something like @AltGr's hashed hooked idea to alert users from the command line. It's been a very common user report since the 2.0 release.

@rjbou rjbou merged commit 1109a14 into ocaml:2.0 Dec 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dra27

This comment has been minimized.

Copy link
Contributor

dra27 commented Dec 16, 2018

An idea to solve this could be to keep a static definition of all known by-default previous configs (or preferably just their hashes). Then when we detect an opam version bump, we can upgrade automatically states that match the known hashes, and just print a warning when we detect a modified or custom version (a little like Debian does for modified configuration files). This could apply both to the script file and to the hooks configuration, allowing for a --disable option in the latter case (assuming we want to provide such an option :/ )

This sounds brittle - especially once you start considering CRLF/LF joy and also third-party opams. For this to work, you need to know what the default configuration would have been when the root was initialised and what the actual configuration used was - why not just record both of those as actual files in the root (with no "actual" configuration file written if the built-in opam default was used). An upgrade of opam can then detect field-by-field whether the defaults have altered and take action accordingly:

  • If the configuration was actually the default, then automatic upgrade seems reasonable
  • If the specific field was the default value, then automatic upgrade is possible reasonable?
  • If the specific field was different, then either a warning or an interactive prompt is wanted?

A similar mechanism could be used to warn about the need to propagate changes if the /etc/opamrc file gets updated.

@avsm

This comment has been minimized.

Copy link
Member

avsm commented Dec 16, 2018

How about just unconditionally showing a motd with the changelog if the version of opam has changed? That would just inform all users of the changes in the release.

@dra27

This comment has been minimized.

Copy link
Contributor

dra27 commented Dec 16, 2018

That would also be a good idea, apart from the unconditionally bit - it should may be display it only once or have the ability to suppress it once you've seen it... if you repeatedly show a message which someone knows will appear, then they stop paying attention completely and don't notice when it changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment