Skip to content
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

CLHEP modulemap for CXXMODULES IB #5

Closed
wants to merge 2 commits into from

Conversation

oshadura
Copy link

@cmsbuild
Copy link

A new Pull Request was created by @oshadura (Oksana Shadura) for branch cms/v2.4.1.3.

@cmsbuild, @smuzaffar, @mrodozov, @tulamor can you please review it and eventually sign? Thanks.
cms-bot commands are listed here

@smuzaffar
Copy link

@oshadura , instead of adding it here which will endup in all our IBs, I would suggest to add it in to cms-sw/cmsdist rootmodule branch by creating a new spec e.g. cxxmodulemaps.spec . In future we can also add modulemaps for other external tools (e.g. boost etc.)

@smuzaffar
Copy link

If no objections then I can create the cxxmodulemaps.spec

@vgvassilev
Copy link

One of the advantages when having the module.modulemap as part of the package is that it will speed up builds with clang -fmodules. Most of the apple sdk comes with these modulemap plus a handful of other unix libraries. I do not have a strong opinion but I can see these commits upstreamed in clhep mainline.

@vgvassilev
Copy link

Before taking any action, I proposed @oshadura to replace the current modulemap with

module clhep {
  export *
  requires cplusplus
  umbrella "CLHEP"
  module * { export *}
}

which is more maintainable and should have the same effect.

@smuzaffar
Copy link

I would still suggest to add this in cmsdist (rootmodule branch) and update clhep.spec to deploy deploy it. e.g something like this in clhep.spec would work

  1. add a new file in cmsdist modulemaps/clhep.modulemap.file
  2. Add the extra source
Source1: modulemaps/clhep.modulemap
  1. deply the file in clhep include at rhe end of %install section
cp %{_sourcedir}/clhep.modulemap %(i}/include/

this way overhead to manage this file for every clhep update will be less.

@smuzaffar
Copy link

One of the advantages when having the module.modulemap as part of the package is that it will speed up builds with clang -fmodules. Most of the apple sdk comes with these modulemap plus a handful of other unix libraries. I do not have a strong opinion but I can see these commits upstreamed in clhep mainline.

all this is still in development, so I do not want to rebuild clhep for our production archs every other day because we want to update the modulemap. Once we have a working modoule map then I would suggest that we request to include it in upstream clhep repository instead of we patch it for every other version.

@smuzaffar
Copy link

cms-sw/cmsdist#5795 shall deploy clhep.modulemap in clhep/include directory now.

@oshadura
Copy link
Author

oshadura commented May 2, 2020

@smuzaffar thank you so much! your and @vgvassilev solution is much more elegant 👍

@oshadura
Copy link
Author

oshadura commented May 2, 2020

I will close this PR in favor of your PR?

@oshadura oshadura closed this May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants