Skip to content

New package variable: opamfile#4402

Merged
rjbou merged 3 commits intoocaml:masterfrom
rjbou:opamfile-loc-var
Oct 27, 2020
Merged

New package variable: opamfile#4402
rjbou merged 3 commits intoocaml:masterfrom
rjbou:opamfile-loc-var

Conversation

@rjbou
Copy link
Copy Markdown
Collaborator

@rjbou rjbou commented Oct 21, 2020

superseeds #4387

@rjbou rjbou added this to the 2.1.0~beta3 milestone Oct 21, 2020
Copy link
Copy Markdown
Member

@AltGr AltGr left a comment

Choose a reason for hiding this comment

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

Thanks! Some concerns about the generality of implementation, and maybe we could find a better name ? (-loc doesn't sound straightforward to me... Why not just opamfile or opam-file ?)

in
OpamFile.OPAM.get_metadata_dir ~repos_roots opam
|> OpamStd.Option.map (fun d ->
OpamFilename.Op.(d//"opam")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, does this work for pinned packages ? Can't the file be named differently?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

for pinned packages, it's stored in the overlay dir, in the file opam, and from repo, its the package dir with name opam.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

aah right, it'll never get back to the original srcdir, we always have a mirror, got it! 👍
maybe worth adding a comment ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's always there, in the current run of opam (repo dir are tmp ones), yes. In fact, it's the code of OpamRepositoryState.get_root, that is not reachable from OpamPackageVar.
+1 for the comment

@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Oct 21, 2020

ok for me to rename it. opamfile can be misleading with the content of the opam file... first I thought about opamfile-path

@lefessan
Copy link
Copy Markdown
Contributor

I think opamfile is ok, nobody would expect the content of the file as an argument to a hook script, no ?

@rjbou rjbou changed the title New package variable: opamfile-loc New package variable: opamfile Oct 26, 2020
@rjbou rjbou merged commit 15e9a7a into ocaml:master Oct 27, 2020
@rjbou rjbou mentioned this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants