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

make pcms for tinyxml and clhep #5862

Merged
merged 3 commits into from May 26, 2020

Conversation

davidlange6
Copy link
Contributor

No description provided.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlange6 (David Lange) for branch IB/CMSSW_11_2_X/rootmodule.

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

@davidlange6
Copy link
Contributor Author

@vgvassilev @smuzaffar
this would need the on-going IB to test - I would plan to merge it tonight (its just in the modules branch...)

pcm_util.spec Outdated

mkdir %{i}/include
cp clhep.pcm %{i}/include/.
cp tinyxml2.pcm %{i}/include/.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly PCM resides in the lib directory, so I would suggest to copy them to lib directory and use LIBDIR in toolfile too.

@vgvassilev , If I remember correctly, there was some restrictions about having libNAME.so and NAME.pcm in same directory (may be that is only for root dictionaries). So in case of these (clhep and tinyxml) PCMs do we need a library with same name in lib directory?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, they should not be connected to any library/dictionary.

Copy link
Contributor

Choose a reason for hiding this comment

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

also I think llvm finds PCMs via CLING_PREBUILT_MODULE_PATH ... right @vgvassilev ? I guess we need to set CLING_PREBUILT_MODULE_PATH in toolfile too

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, usually we do. In those particular case tinyxml2 and clhep depend only on libc and std (afaik) which will be loaded by default by rootcling -- we should be safe without that variable I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

without the variable how will llvm know about these pcms?

Copy link
Contributor

Choose a reason for hiding this comment

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

for cmssw, we set CLING_PREBUILT_MODULE_PATH (pointing to CMSSW_BASE/lib/arch:CMSSW_RELEASE/BASE/lib/arch ) so that already build pcms are visible to clang.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smuzaffar - how do I add a PATH like variable to the tool file? Or does scram magically know that this variable is a PATH like variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

without the variable how will llvm know about these pcms?

If you mean how rootcling will find libc.pcm and std.pcm -- you are right in that case we need that variable pointing to the location of those.

Copy link
Contributor

Choose a reason for hiding this comment

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

@smuzaffar - how do I add a PATH like variable to the tool file? Or does scram magically know that this variable is a PATH like variable?

just like https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_11_2_X/master/clhep-toolfile.spec#L20 i.e. add type="path"

I think you do not need LIBDIR or LD_LIBRARY_PATH for this tool. Just add

<runtime name="CLING_PREBUILT_MODULE_PATH" value="$PCM_UTIL_BASE/lib" type="path"/>

@davidlange6
Copy link
Contributor Author

davidlange6 commented May 26, 2020 via email

@cmsbuild
Copy link
Contributor

Pull request #5862 was updated.

@davidlange6
Copy link
Contributor Author

I pushed changes that should address the feedback. What did I miss?

<tool name="pcm_util" version="@TOOL_VERSION@">
<client>
<environment name="PCM_UTIL_BASE" default="@TOOL_ROOT@"/>
<runtime name="CLING_PREBUILT_MODULE_PATH" value="$PCM_UTIL_BASE/lib" type="path"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

@davidlange6 this should be outside the <client></client> block :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

trying again

@cmsbuild
Copy link
Contributor

Pull request #5862 was updated.

@smuzaffar
Copy link
Contributor

smuzaffar commented May 26, 2020

test parameters

  • full_cmssw = true

@smuzaffar
Copy link
Contributor

please test
modules IBs is available now

@cmsbuild
Copy link
Contributor

cmsbuild commented May 26, 2020

The tests are being triggered in jenkins.
Test Parameters:

@cmsbuild
Copy link
Contributor

-1

Tested at: 7a9097d

CMSSW: CMSSW_11_2_CXXMODULE_X_2020-05-25-2300
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-66b37a/6587/summary.html

I found follow errors while testing this PR

Failed tests: Build

  • Build:

I found compilation error when building:

------- copying files from src/CondCore/DBOutputService/scripts -------
>> copied cmscond_logdb_dump
Entering library rule at src/CondCore/DBOutputService/test
>> Building  edm plugin tmp/slc7_amd64_gcc820/src/CondCore/DBOutputService/test/IOVPayloadEndOfJob/libIOVPayloadEndOfJob.so
/cvmfs/cms-ib.cern.ch/nweek-02630/slc7_amd64_gcc820/external/gcc/8.2.0-bcolbf/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.4.0/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCondFormatsBeamSpotObjects
collect2: error: ld returned 1 exit status
gmake: *** [tmp/slc7_amd64_gcc820/src/CondCore/DBOutputService/test/IOVPayloadEndOfJob/libIOVPayloadEndOfJob.so] Error 1
Leaving library rule at src/CondCore/DBOutputService/test
Entering library rule at src/CondCore/DBOutputService/test
>> Building  edm plugin tmp/slc7_amd64_gcc820/src/CondCore/DBOutputService/test/writeBlobComplex/libwriteBlobComplex.so
/cvmfs/cms-ib.cern.ch/nweek-02630/slc7_amd64_gcc820/external/gcc/8.2.0-bcolbf/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.4.0/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCondFormatsBeamSpotObjects


@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

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