bob icon indicating copy to clipboard operation
bob copied to clipboard

scm: natively support patch sets

Open jkloetzke opened this issue 4 years ago • 6 comments

Currently patches are applied with a checkoutScript which has a number of drawbacks:

  • quilt patch series files are not easily supported
  • The user is not able to override the patch set with scmOverrides
  • Changes to the patch set are considered incompatible changes and will trigger a clean rebuild of the dependencies because the checkoutScript has changed.
  • The checkoutScript has to deal with updates to the underlying checkout. This typically fails if a git repository is patched (and there are updates on a branch.) or if a tarball was updated.

jkloetzke avatar Jun 10 '20 06:06 jkloetzke

What do you think about committing the patches to a local 'bob-patched' branch?

If doing so it's clear what changes are made by the patches and which are done additionally. This makes updating the SCM easy as we could rebase - at least for git. Don't know how to handle this in svn or cvs. And the repo is clean in bob status and audit.

rhubert avatar Jul 02 '20 06:07 rhubert

@rhubert How would you handle patches applied to tarballs then?

Ferruck avatar Jul 02 '20 06:07 Ferruck

Maybe git was a bit misleading in the ticket description. In general one should never patch a real SCM like git. I'm more concerned about "url" SCMs.

The idea that I have in mind is that Bob known where patches and SCMs "collide". So when a SCM is updated the patches are first reverted, then the SCM is updated and the patches are re-applied. Typically extracted tarballs are patched and there it currently totally fails if the tarball is updated.

In general the solution should be independent of any of the other SCMs IMHO.

jkloetzke avatar Jul 02 '20 07:07 jkloetzke

@rhubert How would you handle patches applied to tarballs then?

for tarballs you can also create a local git repo ... :see_no_evil: only for patched git's....

In general one should never patch a real SCM like git.

Well ... that's heavily used when your're working with patch sets to support different products / versions / variants...

checkoutSCM:
 - scm: git
   url: //foo
   tag: v${COMPONENT_VERSION}

checkoutVars: [COMPONENT_VERSION, PROJECT]
checkoutScript: |
  if [[ "${COMPONENT_VERSION} == "1.0" ]] then
     patchApply v1.0-*.patch
   else if [[ "${COMPONENT_VERSION} == "2.0" ]] 
     patchApply v2.0-*.patch
   fi;

   if [[ "$PROJECT" == "Project42" ]] then
     patchApply project42.patch
   fi;

   git checkout -b bob-patched
   git add .
   git commit -m "bob: applied patches"

The git repo is just a clean upstream repo but needs to be patched. For v1.0 there are patches a,b,c and for v1.1 d,e,f. In addition to this patch-d is needed build for product_xy.

The problem with the last 3 git lines is, that it's still marked as switched in bob status... but you can clearly see what came from patches and which additional changes you have done...

rhubert avatar Jul 02 '20 09:07 rhubert

But isn't the bob status ... just honest? I mean, this state of the git repository is not upstream. The checkoutScript created a state that cannot be compared against a reference. I would generally suggest to track these patches in the git repository as separate branches. But there may be reasons why this is not practical that I'm not aware of.

I think even if Bob supports patches natively this won't solve the scenario above. The repository will always be dirty compared to the upstream server. Also in case of svn and cvs it is not even possible to commit locally. They always need the server to commit.

jkloetzke avatar Jul 02 '20 09:07 jkloetzke

It depends... If you see the status as 'has anything changed since checkout' than the expectation could be to get 'clean' if there hasn't been changes since checkoutSCM and checkoutScript have been run... It's surprising that it's modified even if you didn't make any changes. ATM the situation is like described above for nearly every recipe making bob status useless. :cry:

But maybe it's something like

checkoutFinialize: True

what I need? The status could be P in this case, maybe with a additional --omit-patched switch. I think this is a different discussion compared to the native patch support.

rhubert avatar Jul 02 '20 10:07 rhubert