<div dir="ltr">satish> Perhaps if some of us get this (create branch) access at <a href="https://github.com/bueler/p4pdes" rel="noreferrer" target="_blank">https://github.com/bueler/p4pdes</a> - the workflow is slightly<div>satish> simplified [and a fork can be avoided which would require toggle of giturl as the MR progresses between</div><div>satish> fork url and upstream url - and the commit-ids that change as the upstream MR progresses..]</div><div><br></div><div>I appreciate the desire to avoid working in forks, so I went ahead and invited Barry, Jed, Satish as collaborators on p4pdes. Please let me know if there are others to invite.</div><div><br></div><div>Indeed, I suppose my preferred workflow on p4pdes is for collaborators to create branches and do MRs. It looks like actually restricting collaborators from pushing on master is not something I can do. ("The ability to restrict branches is a type of branch protection that's available for public and private repositories owned by organizations in GitHub Team, GitHub Enterprise Cloud, and GitHub Enterprise Server.") Other forms of protecting master look like they add steps to my own (direct and minimal) workflow on master.</div><div><br></div><div>Ed</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Nov 2, 2020 at 6:32 AM Barry Smith <<a href="mailto:bsmith@petsc.dev">bsmith@petsc.dev</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On Nov 2, 2020, at 12:39 AM, Satish Balay <<a href="mailto:balay@mcs.anl.gov" target="_blank">balay@mcs.anl.gov</a>> wrote:<br>
> <br>
> On Sun, 1 Nov 2020, Barry Smith wrote:<br>
> <br>
>> <br>
>> Model: When working with PETSc master.<br>
>> <br>
>> The tests in a "master" (not necessarily the exact master that the external person is using) of the external package run in the CI.<br>
>> <br>
>> Goal: The PETSc developer can debug and make changes to PETSc and to the external package to fix the programs in the most painless way possible. With the least number of steps, MR, and generally forgetting things.<br>
>> <br>
>> Example: I change some PETSc code and the pipeline comes back with a failure on a p4pdes example.<br>
>> <br>
>> I debug the crashing p4pdes example and fix a problem in it.<br>
>> <br>
>> I now want to make a PETSc MR with my new PETSc code and update the copy of the external "master" so when my MR get's merged the pipelines will work with the external "master". <br>
> <br>
> We are still missing how we are syncing your fixes with p4pdes upstream. MR to upstream? (and likely it will have to through more revisions than what we had with slepc/petsc before getting merged.)<br>
<br>
We decide in advance if Ed wants it every fix, unlikely, or a monthly, or at release MR. (This can be adjusted at any time). Yes we just make a MR to Ed's repository, he changes his repository based on the MR and we merge his new master into our fork. I don't see this as a big deal, no need for iterations since Ed decides how he wants his code to look.<br>
> <br>
> Note: if we are replicating the slepc [or prior petsc4py] workflow - then: the PETSc MR does not get merged until the upstream MR is merged. (so need this coordination) Is this acceptable with p4pdes?<br>
<br>
No, I don't think we should do this with SLEPc either, it puts too much burden on Jose and slows things down. We should always point to our fork, never directly to p4pdes or SLEPc so everyone can work at their own pace asynchronously. Releases point to the true repository when possible, master points to the forks.<br>
<br>
> <br>
> And then there is the permissions issue. We had permissions to create branches in petsc4py repo - so could create these MRs easily (and have them compatible with --download-package). [don't remember if we have this access for slepc - but issues where slepc broke and need fixes on slepc side were rare - so we are able to wait for Jose's fix - before merging PETSc.]<br>
<br>
Since we will be doing this for other packages that won't give us permission I would always use a fork even when we don't have to. A fork of p4pdes, a fork of slepc, <br>
> <br>
> Perhaps if some of us get this (create branch) access at <a href="https://github.com/bueler/p4pdes" rel="noreferrer" target="_blank">https://github.com/bueler/p4pdes</a> - the workflow is slightly simplified [and a fork can be avoided which would require toggle of giturl as the MR progresses between fork url and upstream url - and the commit-ids that change as the upstream MR progresses..]<br>
<br>
If some people want to make the MR directly off the repository and have write permission they would just set the original origin to be the fork and when they want to make an MR then push to the remote that is the true repository. I don't think we want or need a model where the "fork" is just the true repository, yes a tiny bit easier for some people some of the time.<br>
<br>
All this is not as complex as you are making out. Basically it just does in a systematic way for all git packages what we do now in an ad hoc way.<br>
<br>
Barry<br>
<br>
> <br>
>> <br>
>> Other Goal: The developer of the external package can update their code asynchronously with our work, that is if they delay we can keep on working on stuff on and on and we can update our test version asynchronously from their work. <br>
> <br>
> yes - this occurs with slepc [and occurred in older petsc4py model]<br>
> <br>
> Satish<br>
> <br>
>> <br>
>> I proposed a plan with --download-xxx which we already are comfortable with and understand. Jed proposed a plan with git tree we are not yet completely familiar with.<br>
>> <br>
>> Barry<br>
>> <br>
>> <br>
>> <br>
>> <br>
>>> On Nov 1, 2020, at 8:56 PM, Satish Balay <<a href="mailto:balay@mcs.anl.gov" target="_blank">balay@mcs.anl.gov</a>> wrote:<br>
>>> <br>
>>> On Sun, 1 Nov 2020, Barry Smith wrote:<br>
>>> <br>
>>>> <br>
>>>> <br>
>>>>> On Nov 1, 2020, at 2:57 PM, Jed Brown <<a href="mailto:jed@jedbrown.org" target="_blank">jed@jedbrown.org</a>> wrote:<br>
>>>>> <br>
>>>>> Barry Smith <<a href="mailto:bsmith@petsc.dev" target="_blank">bsmith@petsc.dev</a>> writes:<br>
>>>>> <br>
>>>>>>>> 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. <br>
>>>>>>>> <br>
>>>>>>>> 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.)<br>
>>>>>>>> <br>
>>>>>>>> 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.<br>
>>>>>>>> <br>
>>>>>>>> For completeness I show the exact the work flow for my suggestion<br>
>>>>>>>> <br>
>>>>>>>> pipelines --download-p4pdes and runs its tests<br>
>>>>>>>> if it breaks the developer uses --download-p4pdes on their system <br>
>>>>>>>> they fix the problem either by fixing PETSc or what is downloaded from the p4pde fork<br>
>>>>>>>> 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 <br>
>>>>>>>> have the fork on their system<br>
>>>>>>>> they put the fix in new branch in the p4pdes fork and push it<br>
>>>>>>>> they edit p4pdes.py and put a new commit in it pointing to their branch in the fork<br>
>>>>>>>> run the pipeline again<br>
>>>>>>>> if fails with p4pdes they do the above again<br>
>>>>>>>> else the PETSc branch gets accepted and merged to master<br>
>>>>>>>> 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<br>
>>>>>>>> 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 <br>
>>>>>>>> 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.<br>
>>>>>>> <br>
>>>>>>> As you've enumerated here, this requires three MRs per change:<br>
>>>>>> <br>
>>>>>> You still don't understand my suggested approach.<br>
>>>>>> <br>
>>>>>>> <br>
>>>>>>> 1. in PETSc with the actual change and to point at a p4pdes commit<br>
>>>>>> <br>
>>>>>> Yes<br>
>>>>>> <br>
>>>>>>> 2. in p4pdes (Ed's or our fork) to implement needed changes (note: this can't merge until #1 merges)<br>
>>>>>> <br>
>>>>>> 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.<br>
>>>>>> 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.<br>
>>>>>> <br>
>>>>>>> 3. in PETSc to point at the merge commit of p4pdes (after #2 merges)<br>
>>>>>> <br>
>>>>>> No, the MR 1 already has the correct new commit the developer put in step one. <br>
>>>>> <br>
>>>>> p4pdes.py either needs a branch name or a commit, but in practice, it needs to be a commit. Why?<br>
>>>>> <br>
>>>>> petsc/master works with p4pdes/master<br>
>>>>> petsc/breaking-change works with p4pdes/breaking-change<br>
>>>>> <br>
>>>>> 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.<br>
>>>>> <br>
>>>>> 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.<br>
>>>>> <br>
>>>>> So p4pdes.py needs to name a commit, not a branch. You said:<br>
>>>>> <br>
>>>>>>>> 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<br>
>>>>>>>> our fork with his latest master with a simple merge of his. This will then become the new one we test against.<br>
>>>>> <br>
>>>>> After any such merge, we need to update p4pdes.py with the merge commit.<br>
>>>>> <br>
>>>>>> <br>
>>>>>> For p4pdes release we can do the same thing but make sure for each change we make an MR for Ed immediately. <br>
>>>>>> <br>
>>>>>>> <br>
>>>>>>> With subtree, there is only one MR and it's in the PETSc repository. <br>
>>>>>> <br>
>>>>>> Please show the whole work flow with gittree<br>
>>>>> <br>
>>>>> Everything is in the PETSc repository and developers don't need to know about the p4pdes repository.<br>
>>>> <br>
>>>> From what I read earlier a single commit can include either something in PETSc and something in petsc subtree but it cannot include both;<br>
>>> <br>
>>> 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.<br>
>>> <br>
>>>> 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.<br>
>>>> <br>
>>>> 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.<br>
>>>> <br>
>>>> If it breaks we can switch to another model.<br>
>>>> <br>
>>>> Barry<br>
>>>> <br>
>>>> I don't know why Lisandro and Satish could not get it working for petsc4py but I fear the same here.<br>
>>> <br>
>>> I've mentioned this in my other e-mail. the subtree model and commands. Currently 'git subtree push' is broken.<br>
>>> <br>
>>> And we should first decide on the workflow wrt constraints and synchronization mode (wrt changes) before fitting in subtree or fork [or other thing].<br>
>>> <br>
>>> 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]<br>
>>> <br>
>>> Also - don't know why we would need subtree with slepc (as I don't know what workflow is appropriate here).<br>
>>> <br>
>>> 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].<br>
>>> <br>
>>> 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?<br>
>>> <br>
>>> 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.<br>
>>> <br>
>>> Satish<br>
>>> <br>
>>>> <br>
>>>> <br>
>>>> <br>
>>>>> Once per release (or whenever Ed feels like pulling), either<br>
>>>>> <br>
>>>>> * Ed runs git subtree push to his repository, OR<br>
>>>>> * Satish or me runs git subtree push to our fork of p4pdes and sends Ed a PR<br>
>>>> <br>
>>> <br>
>> <br>
> <br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Ed Bueler<br>Dept of Mathematics and Statistics<br>University of Alaska Fairbanks<br>Fairbanks, AK 99775-6660<br>306C Chapman<br></div></div></div></div></div></div></div>