search

cms-sw / cms-git-tools

CMS Git Helpers
34 stars 26 forks source link

use current branch as default new_base for rebase #106

Closed kpedro88 closed 5 years ago

kpedro88 commented 5 years ago

It was brought to my attention a while ago that git-cms-rebase-topic behaves unlike its sibling commands. If git-cms-merge-topic is used twice with branches A and B, the result is base+A+B. If git-cms-rebase-topic is used twice, it creates two separate rebased branches A' = base+A and B' = base+B. This is not obvious or intuitive for most naive use of the command.

Therefore, I have updated the command to use the current branch as the default new base for the rebase. This means that using it twice will result in base+A+B (A rebased onto base, B rebased onto base+A).

The old behavior, if desired, can be restored with the argument --new-base $CMSSW_VERSION. (I suspect most people were not relying on this old behavior, so changing the default should be fine.)

cmsbuild commented 5 years ago

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

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

kpedro88 commented 5 years ago

please test

cmsbuild commented 5 years ago

The tests are being triggered in jenkins. https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3105/console Started: 2019/10/22 23:38

cmsbuild commented 5 years ago

-1

Tested at: 9ce1454f2ca1c5716873eb99a47beb0814403413

You can see the results of the tests here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec94a/3105/summary.html

I found follow errors while testing this PR

Failed tests: Build

I found compilation error when building: 

>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/src/DDSpecparRegistry.cc 
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/src/DDShapes.cc 
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/src/DDAlgoArguments.cc 
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/src/DDFilteredView.cc:1:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/interface/DDFilteredView.h: In member function 'bool cms::DDFilteredView::isA() const':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/interface/DDFilteredView.h:126:22: error: 'instanceOf' is not a member of 'dd4hep'
       return dd4hep::instanceOf(solid());
                      ^~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2019-10-22-1100/src/DetectorDescription/DDCMS/interface/DDFilteredView.h:126:22: note: suggested alternative: 'isInstance'
       return dd4hep::instanceOf(solid());
                      ^~~~~~~~~~
cmsbuild commented 5 years ago

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

kpedro88 commented 5 years ago

um... it seems unlikely that the failure is caused by this PR. is the IB broken?

mrodozov commented 5 years ago

yeah we had some trouble with dd4hep update. it's fixed now

mrodozov commented 5 years ago

please test

cmsbuild commented 5 years ago

The tests are being triggered in jenkins. https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3112/console Started: 2019/10/23 12:56

cmsbuild commented 5 years ago

+1 Tested at: 9ce1454f2ca1c5716873eb99a47beb0814403413 https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec94a/3112/summary.html

cmsbuild commented 5 years ago

Comparison job queued.

cmsbuild commented 5 years ago

Comparison is ready https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec94a/3112/summary.html

Comparison Summary:

smuzaffar commented 5 years ago

+externals

cmsbuild commented 5 years ago

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

smuzaffar commented 5 years ago

@kpedro88 , @fabiocos , we see the IBs are failing[a] and I guess this is related to this change. If I get git-cms-checkout-topic from /cvmfs/cms.cern.ch then I do not get the error. Do you agree thsi the error is related to this change and how should we fix it?

Running scram project... DONE
Running cms-checkout-topic...
pwd; ls; eval `scram run -sh` && which git-cms-checkout-topic && git cms-checkout-topic --ssh CMSSW_11_0_X_2019-10-27-2300 && git cms-addpkg -y FWCore/Version && git cms-checkdeps -A -a
Cloning into '/data/cmsbld/x/CMSSW_11_0_X_2019-10-27-0000/src'...
remote: Enumerating objects: 1, done.
remote: Counting objects: 100% (1/1), done.
remote: Total 2 (delta 1), reused 1 (delta 1), pack-reused 1
Receiving objects: 100% (2/2), done.
Resolving deltas: 100% (1/1), completed with 1 local object.
Checking connectivity: 1959051, done.
Switched to a new branch 'from-CMSSW_11_0_X_2019-10-27-0000'
remote: Enumerating objects: 333, done.
remote: Counting objects: 100% (333/333), done.
remote: Total 1464 (delta 333), reused 333 (delta 333), pack-reused 1131
Receiving objects: 100% (1464/1464), 1.27 MiB | 3.67 MiB/s, done.
Resolving deltas: 100% (471/471), completed with 58 local objects.
From github.com:cmsbuild/cmssw
 * [new branch]              CMSSW_4_1_X         -> my-cmssw/CMSSW_4_1_X
 * [new branch]              CMSSW_4_4_X         -> my-cmssw/CMSSW_4_4_X
 * [new branch]              CMSSW_5_2_X         -> my-cmssw/CMSSW_5_2_X
 * [new branch]              CMSSW_5_3_X         -> my-cmssw/CMSSW_5_3_X
 * [new branch]              CMSSW_6_1_X_SLHC    -> my-cmssw/CMSSW_6_1_X_SLHC
 * [new branch]              CMSSW_6_2_X         -> my-cmssw/CMSSW_6_2_X
 * [new branch]              CMSSW_6_2_X_SLHC    -> my-cmssw/CMSSW_6_2_X_SLHC
 * [new branch]              CMSSW_7_0_X         -> my-cmssw/CMSSW_7_0_X
 * [new branch]              add-condition-for-oracle-test -> my-cmssw/add-condition-for-oracle-test
 * [new branch]              fix-missing-headers-Geometry/HcalCommonData -> my-cmssw/fix-missing-headers-Geometry/HcalCommonData
 * [new branch]              fix-relval-arm-recolocaltrk-sipix-pmaksim-edition -> my-cmssw/fix-relval-arm-recolocaltrk-sipix-pmaksim-edition
 * [new branch]              fix-wf-commands     -> my-cmssw/fix-wf-commands
 * [new branch]              from-CMSSW_10_2_X_2018-04-29-0000 -> my-cmssw/from-CMSSW_10_2_X_2018-04-29-0000
 * [new branch]              gh-pages            -> my-cmssw/gh-pages
 * [new branch]              imported-CVS-HEAD   -> my-cmssw/imported-CVS-HEAD
 * [new branch]              remove-unittest-fpe -> my-cmssw/remove-unittest-fpe
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
/data/cmsbld/x/CMSSW_11_0_X_2019-10-27-0000
biglib
bin
cfipython
config
doc
include
lib
logs
objs
python
src
static
test
tmp
/cvmfs/cms-ib.cern.ch/nweek-02600/common/git-cms-checkout-topic

Traceback (most recent call last):
  File "./pkgtools/cmspm", line 623, in <module>
    main()
  File "./pkgtools/cmspm", line 620, in main
    cmd(args[1:])
  File "./pkgtools/cmspm", line 557, in cmd_frombase
    popen('pwd; ls; eval `scram run -sh` && which git-cms-checkout-topic && git cms-checkout-topic --ssh %s && git cms-addpkg -y FWCore/Version && git cms-checkdeps -A -a' % (release), False, True)
  File "./pkgtools/cmspm", line 262, in popen
    raise CalledProcessError(p.returncode, ' '.join(args))
subprocess.CalledProcessError: Command 'p w d ;   l s ;   e v a l   ` s c r a m   r u n   - s h `   & &   w h i c h   g i t - c m s - c h e c k o u t - t o p i c   & &   g i t   c m s - c h e c k o u t - t o p i c   - - s s h   C M S S W _ 1 1 _ 0 _ X _ 2 0 1 9 - 1 0 - 2 7 - 2 3 0 0   & &   g i t   c m s - a d d p k g   - y   F W C o r e / V e r s i o n   & &   g i t   c m s - c h e c k d e p s   - A   - a' returned non-zero exit status 128
fabiocos commented 5 years ago

@smuzaffar I am unable to trivially reproduce this issue, and another unrelated test seems to work correctly. Anyway I understand that there are problems observed in the week end with IBs that could be partly related to this. I see anyway the IB is now running

fabiocos commented 5 years ago

I understand that this works just because of a temporary workaround anyway...

kpedro88 commented 5 years ago

@fabiocos @smuzaffar I can reproduce the issue and the fix is in #107

smuzaffar commented 5 years ago

@kpedro88 , do we need to pass --new-base $CMSSW_VERSION now?

smuzaffar commented 5 years ago

note that this ( https://github.com/cms-sw/pkgtools/blob/V00-32-XX/cmspm#L555 ) is the code which we run when build the patch release

kpedro88 commented 5 years ago

@smuzaffar --new-base is only used by the rebase-topic version of the command. checkout-topic broke because of an attempt to use a git command while outside of $CMSSW_BASE/src