add cms-rmpkg tool

[kpedro88]

This new tool follows the same approach as #79, where the existing script (cms-addpkg) behaves differently depending on the command name. It required some refactoring here, but I think the end result is reasonably concise.

I also made sure the script refers to $CMSSW_BASE/src/.git consistently - some commands just used .git, which could inconsistencies.

[cmsbuild]

A new Pull Request was created by @kpedro88 (Kevin Pedro) for branch master.

@cmsbuild, @smuzaffar, @iahmad-khan, @davidlange6 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.

external issue cms-sw/cmsdist#2960

[fwyzard]

hi @kpedro88, I will read the code properly next week, but I had a question: do you deal with the case where one does

git cms-addpkg FWCore

followed by

git cms-rmpkg FWCore/Services

?

[kpedro88]

No, I hadn't considered adding a whole subsystem. I'll think about how to handle that...

[kpedro88]

Handling any combination of operations that could include subsystems or packages becomes rather complicated. It seems like the most efficient way would be expanding any requested subsystem into all of its packages, so the sparse checkout file only ever includes packages. Do you think it's sufficient to use CMSSW_RELEASE_BASE to make the list of packages? (This wouldn't handle new packages that were just created by the user, but I'm not sure if there's any way to do that.)

[kpedro88]

Actually, after some more thought I've changed my mind. It's best for the tools to account for any branch changes, etc. as well as possible. I'll work on setting up the logic for all the possible cases...

[cmsbuild]

Pull request #80 was updated.

external issue cms-sw/cmsdist#2960

[kpedro88]

This new commit should handle all the possible cases (at least I haven't been able to break it so far).

One weird feature: a chain of commands like this:

git cms-addpkg Configuration
git cms-rmpkg Configuration/Geometry
git cms-rmpkg Configuration

leaves the .gitignore looking like this:

/.gitignore
!/Configuration/Geometry/

but I think this is a similar result to adding a package and then adding the corresponding subsystem, so it's probably not worth trying to handle it explicitly. It won't actually harm any subsequent commands (adding the subsystem again will wipe out the exclusion line).

[cmsbuild]

Pull request #80 was updated.

external issue cms-sw/cmsdist#2960

[kpedro88]

@smuzaffar also updated man pages here

[kpedro88]

@smuzaffar @davidlange6 I just noticed that cms-checkdeps has its own routine to update the sparse checkout file. To make sure the file is kept in a consistent state accounting for possible actions of the new cms-rmpkg tool, maybe cms-checkdeps should just call cms-addpkg?

[smuzaffar]

@kpedro88 , sounds reasonable

[smuzaffar]

@kpedro88 , for building patch release we run following commands. After your changes to cms-addpkg these do not work any more. Can you please check why it is failing ?

BASE_REL=CMSSW_9_2_DEVEL_X_2017-05-10-1100
PATCH_REL=CMSSW_9_2_DEVEL_X_2017-05-11-1100
scram p $BASE_REL
cd $BASE_REL/src/
cmsenv
git cms-addpkg -y FWCore/Version
git checkout $PATCH_REL
git cms-sparse-checkout --exclude-cvs-keywords $BASE_REL $PATCH_REL
git read-tree -mu $PATCH_REL
git cms-checkdeps -A -a

The error message I get is fatal: ref HEAD is not a symbolic ref which is generated from https://github.com/cms-sw/cms-git-tools/blob/master/git-cms-addpkg#L225

[kpedro88]

@smuzaffar I thought you reverted this because of other problems with rmpkg?

I guess the problem is that in the old version of addpkg, it had this:

git read-tree -mu HEAD
CURRENT_BRANCH=`git symbolic-ref --short HEAD`

The rmpkg mode needs to check if packages exist before it tries to remove them, so I moved this variable initialization before the read-tree call. You could do that call twice to make sure HEAD is a known ref.

[smuzaffar]

I removed the symlink cms-rmpkg but kept the changes in addpkg. Ok let me remove/comment the https://github.com/cms-sw/cms-git-tools/blob/master/git-cms-addpkg#L225 and add back it after git read-tree -mu HEAD

[kpedro88]

No, don't remove or comment that line, it will break other things.

You should add another git read-tree -mu HEAD right before that line.

[smuzaffar]

no still fails

++ git status --porcelain --untracked=no
++ grep '^[ACDMRU]'
+ '[' '' ']'
+ git read-tree -mu HEAD
++ git symbolic-ref --short HEAD
fatal: ref HEAD is not a symbolic ref
+ CURRENT_BRANCH=

[kpedro88]

I just tried this:

source /cvmfs/cms-ib.cern.ch/week1/cmsset_default.csh
scram p CMSSW_9_2_DEVEL_X_2017-05-10-1100
cd CMSSW_9_2_DEVEL_X_2017-05-10-1100/src/
cmsenv
git cms-addpkg -y FWCore/Version

and it worked fine... are you sure this is reproducible and not some issue with the git version or something?

[smuzaffar]

try the instructions I wrote

[smuzaffar]

If I revert https://github.com/cms-sw/cms-git-tools/pull/81/commits/c8bcc1d74777e7a0db3956bcb2c75af85b697fb0 (i.e. drop cms-addpkg usage in cms-checkdeps) then things work

[kpedro88]

Ah, yes, I understand now. Just a minute...

[kpedro88]

Let me make a possibly controversial suggestion... you could simplify your patch-release build commands dramatically now:

BASE_REL=CMSSW_9_2_DEVEL_X_2017-05-10-1100
PATCH_REL=CMSSW_9_2_DEVEL_X_2017-05-11-1100
scram p $BASE_REL
cd $BASE_REL/src/
cmsenv
git cms-checkout-topic $PATCH_REL
git cms-addpkg -y FWCore/Version

With my new command, you avoid the detached HEAD state which causes the symbolic ref problem.

Otherwise, just revert #81.

[kpedro88]

(git checkout $TAG causes detached head. cms-checkout-topic does git checkout -b $LOCAL_BRANCH $GITHUB_USER/$BRANCH to make a local branch following the tag, avoiding detached head.)

[smuzaffar]

git cms-checkout-topic $PATCH_REL
git cms-addpkg -y FWCore/Version

worked fine. I will put that in DEVEL IBs for couple of weeks (for more testing of cms-checkout-tpic) before including it for production releases/IBs

[kpedro88]

Great! Though I realized you might still need to do git cms-checkdeps -A -a afterward, since the cms-merge-topic/cms-checkout-topic call to cms-checkdeps doesn't use -A. We could consider adding an option for that to cms-checkout-topic.

(I am teaching a Git/GitHub/CMSSW tutorial on June 12, so it would be great if cms-checkout-topic was publicly available by then.)

[smuzaffar]

ah right ... I will add git cms-checkdeps -A -a If every thing goes fine then hopefully cms-checkout-topic should be available by the end of MAY.