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

Fff in release #662

Closed
wants to merge 6 commits into from
Closed

Fff in release #662

wants to merge 6 commits into from

Conversation

emeschi
Copy link
Contributor

@emeschi emeschi commented Aug 29, 2013

Replaces pull request #652 by aspataru

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @emeschi for CMSSW_7_0_X.

Fff in release

It involves the following packages:

EventFilter/Utilities

@mommsen, @emeschi can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@cmsbuild
Copy link
Contributor

The following categories have been signed by meschi (a.k.a. @emeschi on GitHub): DAQ

@cms-git-daq

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests.

@ktf
Copy link
Contributor

ktf commented Aug 29, 2013

@nclopezo can you please test this?

@nclopezo
Copy link
Contributor

Hi,

I am trying to compile these changes on top of CMSSW_7_0_X_2013-08-29-0200, but I am having the following error:

In file included from /build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/SealModule.cc:2:0:
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:39:64: error: expected class-name before '{' token
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:49:25: error: 'xgi' has not been declared
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:49:36: error: expected ',' or '...' before '*' token
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:62:33: error: 'xdata::InfoSpace' has not been declared
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:63:39: error: 'xdata::InfoSpace' has not been declared
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:94:5: error: 'InfoSpace' in namespace 'xdata' does not name a type
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-29-0200/src/IORawData/DaqSource/plugins/DaqSource.h:95:5: error: 'InfoSpace' in namespace 'xdata' does not name a type
gmake: *** [tmp/slc5_amd64_gcc472/src/IORawData/DaqSource/plugins/IORawDataDaqSourcePlugins/SealModule.o] Error 1

You can see the complete logs here:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc472/319/console

@emeschi
Copy link
Contributor Author

emeschi commented Aug 30, 2013

Hi
well this has nothing to do with pull request #662 but rather with the deprecation of xdaq.
I am surprised the dependence wasn't spotted automatically...
All packages (I think IORawData/DaqSource had been mentioned) which have dependencies on xdaq should be deprecated along with it.
I propose to remove DaqSource from the release right away as well. @ktf if you tell me how to do :) it I will take care of that. I will come up with a replacement for the functionality we still need later today

@emeschi
Copy link
Contributor Author

emeschi commented Aug 30, 2013

Hey experts:
Can I update this pull request with the additional commit above or should I create another topic ?
Emilio

@nclopezo
Copy link
Contributor

Hi @emeschi

You can push the commit to the branch and the pull request will update automatically. I saw that you did it already.

@cmsbuild
Copy link
Contributor

Pull request #662 was updated. Signatures reset, please check and sign again.

@emeschi
Copy link
Contributor Author

emeschi commented Aug 30, 2013

Thank you @nclopezo !
Since I am only two days old on github, can you also tell me how I remove the whole of IORawData/DaqSource so that we can move on with this ?
Sorry for being thick. Thank you.

@ktf
Copy link
Contributor

ktf commented Aug 30, 2013

Ciao Emilio

git cms-addpkg IORawData/DaqSource
git rm -rf IORawData/DaqSource
git push my-cmssw HEAD:remove-daq-source

should do the work.

G.

@cmsbuild
Copy link
Contributor

Pull request #662 was updated. Signatures reset, please check and sign again.

@nclopezo
Copy link
Contributor

Hi @emeschi

I tried to build again but I got the following error:

In file included from /build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelInformationExtractor.cc:9:0:
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/interface/SiPixelEDAClient.h:25:70: error: expected class-name before '{' token
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/interface/SiPixelEDAClient.h:34:16: error: 'xdata' has not been declared
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/interface/SiPixelEDAClient.h:34:33: error: expected ',' or '...' before '*' token
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelEDAClient.cc: In constructor 'SiPixelEDAClient::SiPixelEDAClient(const edm::ParameterSet&)':
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelEDAClient.cc:60:3: error: class 'SiPixelEDAClient' does not have any field named 'ModuleWeb'
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelEDAClient.cc: At global scope:
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelEDAClient.cc:341:6: error: prototype for 'void SiPixelEDAClient::defaultWebPage(xgi::Input*, xgi::Output*)' does not match any in class 'SiPixelEDAClient'
In file included from /build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/src/SiPixelEDAClient.cc:1:0:
/build/cmsbuild/jenkins-workarea/workspace/Pull-Request-Integration/ARCHITECTURE/slc5_amd64_gcc472/CMSSW_7_0_X_2013-08-30-0200/src/DQM/SiPixelMonitorClient/interface/SiPixelEDAClient.h:32:8: error: candidate is: void SiPixelEDAClient::defaultWebPage(int)
gmake: *** [tmp/slc5_amd64_gcc472/src/DQM/SiPixelMonitorClient/src/DQMSiPixelMonitorClient/SiPixelEDAClient.o] Error 1

You can see the logs here:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc472/326/consoleFull

@emeschi
Copy link
Contributor Author

emeschi commented Aug 30, 2013

Hi @nclopezo
yes that is only to be expected...
this package was also dependent on xdaq and should have been removed as discussed in
https://hypernews.cern.ch/HyperNews/CMS/get/database/1226.html
It appears that the relevant people have not yet done that.
These are not part of the DAQ realm whatsoever so they should be taken care of by the AlCA people

@emeschi
Copy link
Contributor Author

emeschi commented Aug 30, 2013

It appears that this is becoming a bit messy...
The removal of xdaq from the externals brought about the breaking of lots of packages as discussed in the hypernews thread. I only signed off for the removal of the relevant EventFilter packages. My goal is to get out the new stuff we need for the integration of F3... not to clean up of xdaq other people's code. I think someone should orchestrate the removal or modification of all the remaining packages which depend on xdaq ahead of that.

@apfeiffer1
Copy link
Contributor

I've just pinged the DB experts again - as this is mainly used in the
online environment, I do not feel expert enough to simply decide on removal
of the corresponding packages (and I'm not sure what would happen with the
overall build once these get removed ... ).

On Fri, Aug 30, 2013 at 2:35 PM, Emilio Meschi notifications@github.comwrote:

It appears that this is becoming a bit messy...
The removal of xdaq from the externals brought about the breaking of lots
of packages as discussed in the hypernews thread. I only signed off for the
removal of the relevant EventFilter packages. My goal is to get out the new
stuff we need for the integration of F3... not to clean up of xdaq other
people's code. I think someone should orchestrate the removal or
modification of all the remaining packages which depend on xdaq ahead of
that.


Reply to this email directly or view it on GitHubhttps://github.com//pull/662#issuecomment-23558026
.

Thanks,
cheers, andreas

@ktf
Copy link
Contributor

ktf commented Sep 9, 2013

Any news about this?

@emeschi
Copy link
Contributor Author

emeschi commented Sep 9, 2013

Giulio,
At any rate, my pull request is independent of whether xdaq is removed or not from the externals.
In fact, it is about removing any dependence from it.
Can we separate my pull request from the removal of xdaq ?
Just for information, xdaq can stay in the cmssw externals, but the DAQ group is dropping any support for building xdaq as an external to cmssw, which means any package dependent on it will eventually be broken by some change in lower-level dependencies of xdaq (e.g. xerces) or the compiler or something else.
Emilio


Emilio Meschi
Senior Physicist
CERN PH/CMD
LHC Programme Coordinator

On Sep 9, 2013, at 5:53 PM, Giulio Eulisse notifications@github.com wrote:

Any news about this?


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Sep 9, 2013

Emilio, I agree. I'll add explicit xdaq dependencies to those packages so that they will not depend on your stuff to get them.

@apfeiffer1
Copy link
Contributor

That is probably a good idea.

In any case, Alexander from HCal (Thanks !) has provided PR 737 (which I
just re-signed after an update), so if this now builds and tests OK, we
have one of the issues fixed.

@ktf
Copy link
Contributor

ktf commented Sep 10, 2013

@nclopezo can you try to see if #760 fixes the compilation issues with this one?
@emeschi the reason why the dependency was not spotted is that you would have had to do git cms-checkdeps -a -b in order to check for buildfile dependencies. I'll discuss with Shahzad to see if there is any particular reason why this is not the default for checkdeps.

@nclopezo
Copy link
Contributor

Hi,

This pull request has now conflicts, so it needs to be rebased. I took this pull request (662) and solved the conflicts manually on my local environment. Then I pulled #760 and created a branch on my own fork of cmssw, you can see the branch here:

https://github.com/nclopezo/cmssw/tree/testing-pr-662-and-760

and the comparison with of official-cmssw/CMSSW_7_0_X here:

https://github.com/nclopezo/cmssw/compare/testing-pr-662-and-760

I then submitted a job to Jenkins but I keep getting several compilation errors, you can see the complete logs here:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc481/437/console

Did I do something wrong?

@emeschi
Copy link
Contributor Author

emeschi commented Sep 10, 2013

Well, as it turns out, some packages not only depend directly on xdaq but they also depend on functionality which was in EventFilter as a results of its dependence from xdaq.
DQM/SiStripMonitorClient, for example, will need a serious look at.
I don't know how we can proceed any further.
Perhaps it is all my fault for not checking dependencies correctly, in which case I apologise.
But from the DAQ standpoint we DO need the content of pull request #662 in a 70X release
at some point. We can live with it for a while more but my impression is that nobody will act
unless we take some disruptive action, like excluding the non-compiling packages.
I don't think I can provide a replacement for the xdaq functionality they were using, and putting back them with their xdaq dependencies is simply not an option...
They should look in the cmssw toolbox for something which provides similar features, or copy over the classes in Utilities
which depend on xdaq, but this will only solve the problem temporarily (i.e. until the xdaq build in cmssw breaks).
Emilio


Emilio Meschi
Senior Physicist
CERN PH/CMD
LHC Programme Coordinator

On Sep 10, 2013, at 5:07 PM, David Mendez notifications@github.com wrote:

Hi,

This pull request has now conflicts, so it needs to be rebased. I took this pull request (662) and solved the conflicts manually on my local environment. Then I pulled #760 and created a branch on my own fork of cmssw, you can see the branch here:

https://github.com/nclopezo/cmssw/tree/testing-pr-662-and-760

and the comparison with of official-cmssw/CMSSW_7_0_X here:

https://github.com/nclopezo/cmssw/compare/testing-pr-662-and-760

I then submitted a job to Jenkins but I keep getting several compilation errors, you can see the complete logs here:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc481/437/console

Did I do something wrong?


Reply to this email directly or view it on GitHub.

@ktf
Copy link
Contributor

ktf commented Sep 10, 2013

Yeah, it looks like it's not so easy. I talked to @rovere earlier and I think we agreed that we can simply drop "complicated" packages from the CMSSW_7_0_X branch and add them back, broken, as topic branches, so that people can work in getting back to work. Marco, @deguio can you look into what it takes to drop DQM/SiStripMonitorClient and DQM/SiPixelMonitorClient from the release?

@rovere
Copy link
Contributor

rovere commented Sep 11, 2013

Ciao @ktf, all,
@danduggan is looking into it right now with high priority.

FYI
@deguio

@ktf
Copy link
Contributor

ktf commented Sep 12, 2013

@danduggan any news about this? I would like to close this by later today.

@rovere
Copy link
Contributor

rovere commented Sep 12, 2013

ciao @ktf,
Dan is there and we are close to finalize the commit, hopefully.

Ciao,
Marco.

@ktf
Copy link
Contributor

ktf commented Sep 13, 2013

Closing this since it was superseded by #814.

@ktf ktf closed this Sep 13, 2013
@ktf ktf mentioned this pull request Sep 14, 2013
clelange pushed a commit to clelange/cmssw that referenced this pull request Nov 17, 2016
Add 80X pileup jet ID and fix 76X pileup jet ID
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

6 participants