Homogenize the behaviour of initial repository loads and subsequent loads from diff - #6924
Homogenize the behaviour of initial repository loads and subsequent loads from diff#6924kit-ty-kate wants to merge 3 commits into
Conversation
f13c4be to
3f057a8
Compare
| begin match read_package_opam ~repo_name ~repo_root full_path with | ||
| | Some opam -> OpamPackage.Map.add nv opam r | ||
| | None -> r |
There was a problem hiding this comment.
We no longer have the error log "ERR: directory name not a valid package: ignored %s" in this version.
There was a problem hiding this comment.
The error cannot happen in the new version since we're checking using get_pkg_dir. Also i'm unable to find this debug log anywhere in our testsuite
| if OpamFilename.Dir.Set.mem pkg_dir processed_dirs then | ||
| match OpamRepositoryPath.get_pkg_dir file with | ||
| | None -> | ||
| log "ERR: file name not a valid package: ignored %s" |
There was a problem hiding this comment.
| log "ERR: file name not a valid package: ignored %s" | |
| log "ERR: directory name not a valid package: ignored %s" |
There was a problem hiding this comment.
Well %s is not a directory but a file, so it should rather be
| log "ERR: file name not a valid package: ignored %s" | |
| log "ERR: file name is not part of a valid package directory: ignored %s" |
| | Some (nv, _pkg_dir) -> | ||
| (* NOTE: we ignore [pkg_dir] because load_opams_from_dir | ||
| will always scan parent directories before going deeper *) | ||
| begin match read_package_opam ~repo_name ~repo_root full_path with |
There was a problem hiding this comment.
we need to check here if the file is "opam", otherwise we take ito account each file that has a valid opam file content (the extrafile did not change)
There was a problem hiding this comment.
mmh i don't think so. Do you have an example where it does that? read_package_opam fails if we give it anything but a directory containing a file named opam.
| @@ -0,0 +1,628 @@ | |||
| N0REP0 | |||
There was a problem hiding this comment.
we need to add the same test with load_from_dir
There was a problem hiding this comment.
and a test for files/files to see the change
| List.filter (fun seg -> | ||
| if seg = ".." then | ||
| invalid_arg "'..' is forbidden" | ||
| else | ||
| seg <> "" && not (String.equal current_dir_name seg)) | ||
| x |
There was a problem hiding this comment.
| List.filter (fun seg -> | |
| if seg = ".." then | |
| invalid_arg "'..' is forbidden" | |
| else | |
| seg <> "" && not (String.equal current_dir_name seg)) | |
| x | |
| match | |
| List.filter (fun seg -> | |
| if seg = ".." then | |
| invalid_arg "'..' is forbidden" | |
| else | |
| seg <> "" && not (String.equal current_dir_name seg)) | |
| x | |
| with | |
| | [] -> [current_dir_name] | x -> x |
2 participants
Alternative to #6871
This enforces:
packages/filessubdirectory (as inpackages/a.1/files/files/file)opamfiles in intermediate directories (e.g.packages/a/opamwill be ignored butpackages/a/a.1/opamwill now be detected). This is useless in practice but makes implementation easier and simplifies things for both load from diff and Improve performance of opam update/init by changing the structure of the internal http opam repositories (use the tar.gz as-is) #6625Should be reviewed using
Hide whitespace