On Sun, 1 Nov 2020, Satish Balay via petsc-dev wrote: > On Sun, 1 Nov 2020, Barry Smith wrote: > > > > > > > > On Nov 1, 2020, at 2:57 PM, Jed Brown <[email protected]> wrote: > > > > > > Barry Smith <[email protected]> writes: > > > > > >>>> I vote to fix things in our fork or gittree thing continuously since > > >>>> it makes it easier to fix things rather than wait to the release when > > >>>> we try to find and fix everything and it also helps tell us if we > > >>>> introduced a real bug into PETSc and fix PETSc immediately instead of > > >>>> waiting up to 6 months, just like we now we test immediately with > > >>>> Petsc4py and we should do with SLEPc. How often we give the updates > > >>>> to Ed is a completely different issue. > > >>>> > > >>>> So again back to my original statement ,it comes down to if the > > >>>> subtree or the fork approach is easier for all the PETSc developers > > >>>> who do not currently know gittree and would need to learn it with your > > >>>> approach. I don't know which is easier learning to use gittree which > > >>>> has its own gotcha's or using mine which we all know but may require > > >>>> an extra step (not involving Ed, just updating the p4pdes.py commit > > >>>> each time we change something in the fork.) > > >>>> > > >>>> I think using --download-p4pdes on a couple of systems in the CI is > > >>>> enough, I don't think we need to put it all CI pipelines (I would like > > >>>> slepc in all pipelines).but we could put in all pipelines if we want. > > >>>> > > >>>> For completeness I show the exact the work flow for my suggestion > > >>>> > > >>>> pipelines --download-p4pdes and runs its tests > > >>>> if it breaks the developer uses --download-p4pdes on their system > > >>>> they fix the problem either by fixing PETSc or what is downloaded > > >>>> from the p4pde fork > > >>>> if the fix is in the p4pdes fork they make a branch in the > > >>>> p4pdes fork, which they already have since they used --download-p4pdes > > >>>> and thus > > >>>> have the fork on their system > > >>>> they put the fix in new branch in the p4pdes fork and push it > > >>>> they edit p4pdes.py and put a new commit in it pointing to > > >>>> their branch in the fork > > >>>> run the pipeline again > > >>>> if fails with p4pdes they do the above again > > >>>> else the PETSc branch gets accepted and merged to master > > >>>> depending on Ed's choice we make an MR for p4pdes depending on > > >>>> the agreed upon cycle. If Ed puts the fix into his master then we just > > >>>> update > > >>>> our fork with his latest master with a simple merge of > > >>>> his. This will then become the new one we test against. If Ed doesn't > > >>>> respond to the MR all is fine we > > >>>> just continue on our fork. If he puts other things in his > > >>>> branch but not our MR we just merge that into our fork and so are > > >>>> still testing with his latest master. > > >>> > > >>> As you've enumerated here, this requires three MRs per change: > > >> > > >> You still don't understand my suggested approach. > > >> > > >>> > > >>> 1. in PETSc with the actual change and to point at a p4pdes commit > > >> > > >> Yes > > >> > > >>> 2. in p4pdes (Ed's or our fork) to implement needed changes (note: this > > >>> can't merge until #1 merges) > > >> > > >> Not a MR just a new branch (or commit) in the fork off the last > > >> branch and the commit that goes into petsc4py from the new branch. > > >> This is our fork of p4pdes and there is no reason to do MR on it. The > > >> MRs are done by Ed based on a schedule Ed picks from here to his repo, > > >> if he doesn't like something we've done he requests changes to MR branch > > >> as always. > > >> > > >>> 3. in PETSc to point at the merge commit of p4pdes (after #2 merges) > > >> > > >> No, the MR 1 already has the correct new commit the developer put in > > >> step one. > > > > > > p4pdes.py either needs a branch name or a commit, but in practice, it > > > needs to be a commit. Why? > > > > > > petsc/master works with p4pdes/master > > > petsc/breaking-change works with p4pdes/breaking-change > > > > > > We can't merge p4pdes/breaking-change to p4pdes/master until > > > petsc/breaking-change merges to petsc/master (otherwise it would break > > > everyone else's workflow). But now petsc/master will point at > > > p4pdes/breaking-change and needs to be updated after the merge. > > > > > > Meanwhile, petsc/bug-fix branched off petsc/master prior to this ordeal > > > and still points at p4pdes/master, which will break after all the merges > > > occur. I guess you could stop the world and tell everyone to rebase or > > > merge from master, but that's even more disruptive. > > > > > > So p4pdes.py needs to name a commit, not a branch. You said: > > > > > >>>> depending on Ed's choice we make an MR for p4pdes depending on > > >>>> the agreed upon cycle. If Ed puts the fix into his master then we just > > >>>> update > > >>>> our fork with his latest master with a simple merge of > > >>>> his. This will then become the new one we test against. > > > > > > After any such merge, we need to update p4pdes.py with the merge commit. > > > > > >> > > >> For p4pdes release we can do the same thing but make sure for each > > >> change we make an MR for Ed immediately. > > >> > > >>> > > >>> With subtree, there is only one MR and it's in the PETSc repository. > > >> > > >> Please show the whole work flow with gittree > > > > > > Everything is in the PETSc repository and developers don't need to know > > > about the p4pdes repository. > > > > From what I read earlier a single commit can include either something in > > PETSc and something in petsc subtree but it cannot include both; > > It can include both. Subtree is a way to import (pull) export (push) external > repo changes. However once imported - its viewed as a single petsc repo. > > > can a new branch in the PETSc repository encompass both repositories. This > > would a less than ideal work flow for developers. Also does a git pull in > > the PETSc dir automatically do the pull on the gittree, from the docs it is > > not so clear. > > > > But we wasted too much time arguing. Since you are so convinced that > > subtree will work painless set it up and let's use it. Also set it up for > > SLEPc. > > > > If it breaks we can switch to another model.
One additional note: once a subtree is imported - its like adding a commit (or commits)- its a permanent record. Removal (if switching to another model) is like adding a delete commit. Satish > > > > Barry > > > > I don't know why Lisandro and Satish could not get it working for > > petsc4py but I fear the same here. > > I've mentioned this in my other e-mail. the subtree model and commands. > Currently 'git subtree push' is broken. > > And we should first decide on the workflow wrt constraints and > synchronization mode (wrt changes) before fitting in subtree or fork [or > other thing]. > > Perhaps there is a workflow with subtree that doesn't involve 'git subtree > push' - in which case we can use subtree. [also I haven't used 'subtree pull' > so don't know if it also has issues] > > Also - don't know why we would need subtree with slepc (as I don't know what > workflow is appropriate here). > > both subtree/fork are suitable for asynchronous changes [like we have with > pkg-mumps etc] where we have changes that are not easy to push to upstream > [some might be accepted - after a delay - and some are not]. > > For synchronous changes - its best to work with the 'primary' repo. Wrt > petsc4py - we used subtree to migrate the primary repo [for petsc4py sources] > to petsc. But slepc? > > Note: with 2 different "primary" repos - there will always be a > synchronization cost - it just moves around [with subtree vs the current mode > of directly using upstream slepc] - the cost is not eliminated. subtree [with > 2 primary repos] - will primarily help with delayed synchronization. > > Satish > > > > > > > > > > Once per release (or whenever Ed feels like pulling), either > > > > > > * Ed runs git subtree push to his repository, OR > > > * Satish or me runs git subtree push to our fork of p4pdes and sends Ed a > > > PR > > >
