ocaml-opam / camelus Goto Github PK

Warning 41

warning 41: Some packages are mentionned in package scripts of features,
but there is no dependency or depopt toward them: "user-setup"

seem to imply that user-setup should be mentioned as a dependency — and indeed doing so makes the warning go away — but doing so creates a circular dependency. An improvement would be welcome.

In this PR: ocaml/opam-repository#16495, the package is put in packages/<pkg>.version/ instead of packages/<pkg>/<pkg>.version/ and Camelus still detects it as a new installable packages (1). Could this be detected?

cc @thomasblanc

It is generally nice to have a human-readable message in the case of package installation failure when a package relies upon depexts. More than the usual "use opam depext" message, this message can specify information like the version of a depext required.

If a typo is made to a built-in variable name like writing or instead of os, Camelus does not warn about it. It should be possible to declare the set of bound-or-potentially-bound variables and have warnings emitted if variables from outside that set are used.

Dear @camelus !

We need your help to make sure that these kind of issue do not pop up in the OCaml opam repository.

In other words {test} and {dev} (and others ?) dependency names should be checked to exist in the repo on opam submissions.

This strategy seems to work every time (at least twice so far) a conflict arises when Camelus is trying to merge new commits + transformations to the 2.0 format to the 2.0.0 branch.
I couldn't find the merge command in the code so I'm not sure how to incorporate this but here are my two cents.

See #11943 or ocaml/opam-repository#11877 (comment) for instance

Camelus is not reporting the removal of ocaml/opam-repository#14582 genprint.0.1. I wonder if it's because of the rename?

See ocaml/opam-repository#14288 - it'd be helpful if Camelus warned about unexpected files in a pull request. The valid patterns would be:

• Root files should probably a generate a highlighted note
• /packages/FOO/BAR should be rejected, unless BAR is a directory or a .gitattributes file (the latter should probably generate a highlighted note too)
• /packages/FOO/FOO.VERSION/BAR should be rejected, unless BAR is opam

After a while, the program seems to fail on the server with "Cannot allocate memory" errors ; I couldn't reproduce locally, but have some results.

• ocp-memprof didn't return any unexpected memory usage or leak ; memory peaks at 70MB when we load the git with all opam files in memory, which seems reasonable.
• I traced system memory usage:
  
  Resident memory (VmRSS) stays very reasonable, however, virtual memory (VmSize) jumps to above 100GB... I guess that's where we hit a wall. This is using Linux's /proc/pid/status.
  @samoht, a clue what's happening ? I am guessing maybe too many mmaps ? Or am I doing something plainly wrong with git or lwt ? The peaks are reached during the opam_files function.

When a maintener uploads a package a.2, there is no easy way to check the difference with the package of the previous software version, namely a.1. Camelus might insert a diff (or a link to the diff).

Changes of archive and/or their checksums happens from time to time on opam-repository. It is usually related to how github creates those archives, but sometimes, a mistake can be made and the archive can change in an unexpected way (e.g. ocaml/opam-repository#15294 (comment)).

I think it would be good to have Camelus give a diff when they change to avoid mistakes in the future.

cc @thomasblanc

Camelus should fail a PR check if the PR adds a package to the wrong location in the repository. See ocaml/opam-repository#8447.

Commands with {with-test} attributes inside run-test doesn't do anything. It would be nice to have a warning in case such attribute is present inside run-test

Otherwise it's not obvious that the source of the bot lives here.

By doing $ opam source --dev on it. It should be able to catch issues like this:

ocaml/opam-repository#11342

cc @avsm

From ocaml/opam-repository#5995:

I find these notifications quite useful, since Travis doesnt send a message when it successfully finishes. There's absolutely no harm in a note from a package submitter saying that they believe that a PR is ready to merge since it passes CI, especially since it sometimes takes multiple pushes to get it right.

I find this very noisy, and I'm considering removing myself from watching this repository. If we follow this pattern we get at minimum of 4 notification per package addition:

1. The initial PR
2. Camelus
3. The PR person says it's all green
4. The merge notification

wouldn't it be possible to change this so that Camelus reports directly only if it fails and otherwise once travis succeded or failed ?

It would be nice to be able to have warnings when new package names are created that match any of a set of regular expressions. This could be used to catch new packages that contain unrecommended phrases such as ocaml. See ocaml/opam-repository#6152.

Seems like opam2 fails when translating opam1 packages with duplicated fields to opam2.
Could we have it detected by Camelus ?

Related to: ocaml/opam-repository#11718

This could potentially go in opam lint but it's dependent on an external data source that can change and should be tracked and may represent repository policy rather than a data type constraint.

Related to ocaml/opam#2224.

I can't seem to build this with either opam-lib 1.2.2 or opam-core/opam-format 2.0~alpha4.

ocaml/opam-repository#6212 had only a desc file but Camelus approved it.

This probably requires opam 2.1 but it might be worth using a development version of the opam lib for this purpose.

cc @thomasblanc

Let's say I'm making a mistake and make a PR adding the package pkg/pkgx.1.0.0 to opam-repository.
Currently Camelus doesn't display any errors to avoid this problem. In particular my mistake was to use a - instead of . to separate between package name and version number but of course ended up creating a new package.
Could Camelus do something to highlight this kind of error ?

see ocaml/opam-repository#14978
opam packages released to opam-repository contain checksums of tarballs to ensure that the same tarball is used for installation as intended by the person doing a release. opam packages which point to a branch / tag are generally not accepted AFAICT.

opam 2.0 introduced a pin-depends stanza to specify dependencies of certain off-trunk dependencies (i.e. a custom version of yyy). I just discovered that the opam-repository contains few packages with pin-depends, which I assume should not be there, applying the same rule above.

I believe "make" → make is an often made comment to PRs. This tool should check that, I think.

-x is useful to find out later what was actually executed.
-e is necessary for errors to be propagated correctly in all cases.

Some care will be required to ensure that the false positive rate is not too high due to variations in sh command line invocations.

Many people want that.

Here are a few relevant discussion occured on the opam-repository tracker.

ocaml/opam-repository#6819
ocaml/opam-repository#4897

Camelus uses the branch from where PR are made from without merging it in master first when it checks for broken packages. Which for some people that don't use opam-repository that much can be a big problem.

For example this PR has one commit on top of a commit made in 2017: ocaml/opam-repository#16158. This example is not the first one I've encountered, it's the first time I bother opening an issue about it. There are plenty of similar examples to take from.

Pull requests to opam-repository show up with info like the screen shot below. It would be helpful to get more info somehow. For example, travis tests provide a link pointing to a log of what was run. If nothing else, at least point to this repo and update the README of this repo to provide more information about what is checked.