Skip to content

Commit

Permalink
add the "link" tag to "pack" actions (fixes #272)
Browse files Browse the repository at this point in the history
Instead of a separate category, "pack" is now another form of linking
like "program", "library", "toplevel" and "output_obj". This fixes the
issue that package(...) tags where not passed at pack-production time,
spotted by Jérémie Dimino in
  <ocaml/opam-repository#11628 (comment)>
  <janestreet/ppx_sexp_conv#20>
More generally, this extends the meaning of all "link" flags to
"pack", which seems to be the correct behavior for all the rules we
inspected.

I tested that this change fixes the original 'nocrypto' issue
(once their _tags is also fixed to provide the right package(..)
option at pack-time).

This is a somewhat invasive change to the behavior of "pack" and
"link" tags, which may affect all user-defined flag declarations. An
alternative to this change would be to just add a specific "package"
rule when the "pack" tag is present, but my understanding is that
linking options quite generally apply to "pack" productions
(that really behave mostly like library-archive production already
under the ("link";"library") regime), so this more invasive change is
more correct: the extra cases where it applies were likely to be small
bugs before.

(I checked that ocb-stubblr is orthogonal to this change and does not
need any update.)
  • Loading branch information
gasche committed Mar 26, 2018
1 parent 0c7f9d5 commit 9cb08c6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
9 changes: 9 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ NEXT_RELEASE:
custom, debug, failsafe, linkall, ccopt(..), cclib(..), rpath(..), ldopt(..)
(Gabriel Scherer, report by Hannes Mehnert, review by whitequark)

- #272: add the "link" tag to "pack" actions
Instead of a separate category, "pack" is now another form of linking
like "program", "library", "toplevel" and "output_obj". This fixes
the issue that package(...) tags where not passed at pack-production
time, spotted by Jérémie Dimino. More generally, this extends
the meaning of all "link" flags to "pack", which seems to be the correct
behavior for all the rules we inspected.
(Gabriel Scherer, original issue diagnosis by Jérémie Dimino)

0.12.0 (11 Nov 2017):
---------------------

Expand Down
8 changes: 4 additions & 4 deletions src/ocaml_compiler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -378,26 +378,26 @@ let byte_debug_library_link_mllib = link_from_file byte_debug_library_link_modul

let byte_pack_modules =
pack_modules [("cmo",["cmi"]); ("cmi",[])] "cmo" "cma" "cma" ocamlc_p
(fun tags -> tags++"ocaml"++"pack"++"byte")
(fun tags -> tags++"ocaml"++"link"++"byte"++"pack")

let byte_pack_mlpack = link_from_file byte_pack_modules

let byte_debug_pack_modules =
pack_modules [("d.cmo",["cmi"]); ("cmi",[])] "d.cmo" "d.cma" "d.cma" ocamlc_p
(fun tags -> tags++"ocaml"++"pack"++"byte"++"debug")
(fun tags -> tags++"ocaml"++"link"++"byte"++"pack"++"debug")

let byte_debug_pack_mlpack = link_from_file byte_debug_pack_modules

let native_pack_modules x =
pack_modules [("cmx",["cmi"; !Options.ext_obj]); ("cmi",[])] "cmx" "cmxa" !Options.ext_lib ocamlopt_p
(fun tags -> tags++"ocaml"++"pack"++"native") x
(fun tags -> tags++"ocaml"++"link"++"native"++"pack") x

let native_pack_mlpack = link_from_file native_pack_modules

let native_profile_pack_modules x =
pack_modules [("p.cmx",["cmi"; "p" -.- !Options.ext_obj]); ("cmi",[])] "p.cmx" "p.cmxa"
("p" -.- !Options.ext_lib) ocamlopt_p
(fun tags -> tags++"ocaml"++"pack"++"native"++"profile") x
(fun tags -> tags++"ocaml"++"link"++"native"++"pack"++"profile") x

let native_profile_pack_mlpack = link_from_file native_profile_pack_modules

Expand Down

0 comments on commit 9cb08c6

Please sign in to comment.