From: Hazen B. <hba...@ma...> - 2014-09-22 16:09:07
|
> Message: 5 > Date: Mon, 22 Sep 2014 15:17:36 +0100 > From: Phil Rosenberg <phi...@ya...> > Subject: [Plplot-devel] Exit calls and memory management > To: PLplot development list <Plp...@li...> > Message-ID: <218...@sm...> > Content-Type: text/plain; charset="windows-1252" > > This one is mostly directed at Alan but probably others will be interested and may have comments. Git might also be a big help here, but I'm not sure how it will work so advice from all appreciated. > > I have been trying to improve memory management in plplot as part of my attempt to try to remove exit calls. To do this I am removing raw pointer arrays and replacing them with a struct which will give us more object oriented access to memory allocation. I think this will be a generally good thing as the struct can have function pointers which will allow trivial resizing and adding elements. It will also be extendible, to allow further functionality similar to std::vector if we want. Arrays will also know their own size. > > However the cost is that arrays in the plstream will all become structs so will need accessing by somearray.mem[index] or maybe somearray.getMem()[index] rather than somearray[index]. This has the potential to break some or all the drivers. Although the fixes might be trivial, I don't have easy access to all the drivers on my windows machine. > > I'm therefore looking for a suggestion of how to proceed. I can transfer stuff to my Linux machine, but I'm not sure how best to do that with git. I could create a fork on github so other developers can contribute, but if I create a public fork then we should merge that into the master branch rather than rebase it which I think is not compliant with our current workflow. I think creating a branch on github (or some other public repository) is the only way that you can proceed if you want others to see what you are doing. Though not perhaps ideal, you should be able to rebase master off a public branch. If you are interested, Hez or I could add you to the PLplot organization on github. Then you could push your branch there. We haven't done anything with it yet, so you'll have to create a the repository first. https://github.com/PLplot -Hazen |
From: Alan W. I. <ir...@be...> - 2014-09-22 19:37:41
|
On 2014-09-22 12:08-0400 Hazen Babcock wrote: > I think creating a branch on github (or some other public repository) is > the only way that you can proceed if you want others to see what you are > doing. Though not perhaps ideal, you should be able to rebase master off > a public branch. Hi Hazen: Since our advice to Phil is completely contradictory, we should get this important question of workflow decided for the case of experimental topic development work (as opposed to the workflow for our more usual less controversial topic development which is normally pushed to origin master as soon as the changes by a developer pass his own tests). And once the workflow for this special case of experimental development topics is decided, we should document it in README.developers. You are much more experienced with git than me. However, I thought that rebasing a public branch was always a bad idea for the reasons I mentioned concerning disappearing commits. I am positive a number of resources I read when we were considering using a rebase workflow mentioned this drawback to that approach. Were those resources overstating the case? If they are right, then for rebase workflows like ours experimental branches should not be made accessible on public git repos since the bad choices afterwards are to rebase them periodically (which loses prior public commits) to keep up with master development or else become out of date with master. Which means some other alternative for sharing experimental development topics must be used such as sending patches to this list. My feeling is that working with such patches is unlikely to be much of a burden since git has such good facilities (format-patch and am) for generating and using such patches. Of course, if after we have gained some substantial experience with the patch method for the special case of sharing experimental development, we find that method is inhibiting our experimental development, we should move to a merge workflow instead like suggested by Brad King. But for now, let's give a good trial (at least 6 months) to our current rebase workflow including its constraint (if the resources I read are not overstating the case) that developers should refrain from sharing their experimental branches via public repos because it leads to bad further choices that typically come back to haunt you with a rebase workflow. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: Hazen B. <hba...@ma...> - 2014-09-22 20:41:49
|
On 9/22/2014 3:37 PM, Alan W. Irwin wrote: > On 2014-09-22 12:08-0400 Hazen Babcock wrote: > >> I think creating a branch on github (or some other public repository) is >> the only way that you can proceed if you want others to see what you are >> doing. Though not perhaps ideal, you should be able to rebase master off >> a public branch. > > Since our advice to Phil is completely contradictory, we should get > this important question of workflow decided for the case of > experimental topic development work (as opposed to the workflow for > our more usual less controversial topic development which is normally > pushed to origin master as soon as the changes by a developer pass his > own tests). And once the workflow for this special case of > experimental development topics is decided, we should document it in > README.developers. > > You are much more experienced with git than me. However, I thought > that rebasing a public branch was always a bad idea for the reasons I > mentioned concerning disappearing commits. I am positive a number of > resources I read when we were considering using a rebase workflow > mentioned this drawback to that approach. > > Were those resources overstating the case? Probably not. However I don't really see a problem with someone pushing a private branch of PLplot to a public repo and asking others to have a look and make suggestions, as long as everyone else understands that it is "read only". Actual collaboration on a branch is going to more complicated and I have no experience in this area using a rebase workflow. Exchanging patches sound like as good as an approach as any. -Hazen |
From: Alan W. I. <ir...@be...> - 2014-09-22 22:01:03
|
On 2014-09-22 16:41-0400 Hazen Babcock wrote: > On 9/22/2014 3:37 PM, Alan W. Irwin wrote: >> On 2014-09-22 12:08-0400 Hazen Babcock wrote: >> >>> I think creating a branch on github (or some other public repository) is >>> the only way that you can proceed if you want others to see what you are >>> doing. Though not perhaps ideal, you should be able to rebase master off >>> a public branch. >> >> Since our advice to Phil is completely contradictory, we should get >> this important question of workflow decided for the case of >> experimental topic development work (as opposed to the workflow for >> our more usual less controversial topic development which is normally >> pushed to origin master as soon as the changes by a developer pass his >> own tests). And once the workflow for this special case of >> experimental development topics is decided, we should document it in >> README.developers. >> >> You are much more experienced with git than me. However, I thought >> that rebasing a public branch was always a bad idea for the reasons I >> mentioned concerning disappearing commits. I am positive a number of >> resources I read when we were considering using a rebase workflow >> mentioned this drawback to that approach. >> >> Were those resources overstating the case? > > Probably not. However I don't really see a problem with someone pushing a > private branch of PLplot to a public repo and asking others to have a look > and make suggestions, as long as everyone else understands that it is "read > only". Actual collaboration on a branch is going to more complicated and I > have no experience in this area using a rebase workflow. Exchanging patches > sound like as good as an approach as any. OK. I have updated README.developers accordingly (commit fb1dcd5). @Hazen and Phil: Does the language in this new paragraph seem clear enough about our options for sharing experimental topic branch work that we are not going to push to origin master immediately? Alan |
From: phil r. <phi...@ya...> - 2014-09-23 10:31:02
|
@Hazen and Alan I think an email went AWOL from Hazen somewhere, but from Alan's reply I guess it probably said that we should not be rebasing public branches. I must confess that I think almost everything I have read about rebasing says do not rebase a public branch. I'm certainly no expert, but it does make me nervous that we are suggesting to fly in the face of the perceived wisdom. I think I would be much more comfortable with accepting that where we need to collaborate on things we and use public branches, we merge these instead of rebasing them. I know this makes for a slightly less tidy history, but such large changes don't happen often. Don't forget it isn't only us that can download a public branch - anyone can download from my repo, find and fix a bug, then make a push request - if we have to say sorry that history doesn't exist we can't accept your push, then it doesn't make us look great. That is of course a very idealistic way of looking at things and I'm all for practicality over idealism. So if you still want to go for the rebase option then I have no problem with that. Just so long as we have considered the potential problems. @Andrew. You are of course correct that these changes in themselves will not remove the exit calls, but my intention is that they will make failed memory allocations easier to deal with - even if it is just that by using an array's internal element count it will avoid buffer overruns even if memory allocation fails - i.e. it removes the need for each reallocation to be checked and the element count be either zero'd or reset to its old value. Regarding the replacement to plexit. Two options were discussed at some point. Either 1) Use the plabort callback set by the user to report error messages or 2)Have an error code and message which we save and add a plgeterror() function to the API, to retrieve these values. I am tending towards 1) for the following reasons: a) Almost nobody ever bothers to actually check the error code, but if they have created a callback then they are more likely to make use of the error. (I am as guilty as that as anyone so I now pepper my code with exception throws to force at least some occurance if something fails) b) For 2 we need to reset the error code for every api call. This isn't too bad, but we also have to make sure we never accidentally reset it by calling an API function internally - that will be a real pain to disentangle and enforce. Some arguments for 2 though are i) If we ever make plplot threadsafe then we need some way to make sure the plabort callback is also threadsafe - but I guess we would need that anyway ii) I'm not sure what would happen if our callback threw an exception - it would probably leave the library in some unresolved state. But that is the case for the callback now too. As you said the error code can be set and checked internally and a function can be used to allow it to be easily set by other bindings. Actually in a lot of cases Plplot can probably carry on regardless - providing the array size has been remembered correctly, hence the advantage of arrays which know their own size. However I am certainly still open to suggestions - I think based on the other comments on this thread I will set up a github repository and people can see what I've done so far and comment and suggest as needed. Phil ________________________________ From: Alan W. Irwin <ir...@be...> To: Hazen Babcock <hba...@ma...>; phil rosenberg <phi...@ya...> Cc: PLplot development list <plp...@li...>; Hezekiah M. Carty <he...@0o...> Sent: Monday, 22 September 2014, 23:00 Subject: Re: [Plplot-devel] Exit calls and memory management On 2014-09-22 16:41-0400 Hazen Babcock wrote: > On 9/22/2014 3:37 PM, Alan W. Irwin wrote: >> On 2014-09-22 12:08-0400 Hazen Babcock wrote: >> >>> I think creating a branch on github (or some other public repository) is >>> the only way that you can proceed if you want others to see what you are >>> doing. Though not perhaps ideal, you should be able to rebase master off >>> a public branch. >> >> Since our advice to Phil is completely contradictory, we should get >> this important question of workflow decided for the case of >> experimental topic development work (as opposed to the workflow for >> our more usual less controversial topic development which is normally >> pushed to origin master as soon as the changes by a developer pass his >> own tests). And once the workflow for this special case of >> experimental development topics is decided, we should document it in >> README.developers. >> >> You are much more experienced with git than me. However, I thought >> that rebasing a public branch was always a bad idea for the reasons I >> mentioned concerning disappearing commits. I am positive a number of >> resources I read when we were considering using a rebase workflow >> mentioned this drawback to that approach. >> >> Were those resources overstating the case? > > Probably not. However I don't really see a problem with someone pushing a > private branch of PLplot to a public repo and asking others to have a look > and make suggestions, as long as everyone else understands that it is "read > only". Actual collaboration on a branch is going to more complicated and I > have no experience in this area using a rebase workflow. Exchanging patches > sound like as good as an approach as any. OK. I have updated README.developers accordingly (commit fb1dcd5). @Hazen and Phil: Does the language in this new paragraph seem clear enough about our options for sharing experimental topic branch work that we are not going to push to origin master immediately? Alan |
From: Alan W. I. <ir...@be...> - 2014-09-23 15:55:52
|
On 2014-09-23 03:30-0700 phil rosenberg wrote: > @Hazen and Alan > I think an email went AWOL from Hazen somewhere, but from Alan's reply I guess it probably said that we should not be rebasing public branches. I must confess that I think almost everything I have read about rebasing says do not rebase a public branch. I'm certainly no expert, but it does make me nervous that we are suggesting to fly in the face of the perceived wisdom. I think I would be much more comfortable with accepting that where we need to collaborate on things we and use public branches, we merge these instead of rebasing them. I know this makes for a slightly less tidy history, but such large changes don't happen often. Don't forget it isn't only us that can download a public branch - anyone can download from my repo, find and fix a bug, then make a push request - if we have to say sorry that history doesn't exist we can't accept your push, then it doesn't make us look great. > That is of course a very idealistic way of looking at things and I'm all for practicality over idealism. So if you still want to go for the rebase option then I have no problem with that. Just so long as we have considered the potential problems. It appears e-mails are still going astray. For example, yesterday, I believe Hazen and I came to consensus on this list concerning this issue, and I wrote that consensus up as an additional paragraph in README.developers and asked for your and Hazen's comments on that paragraph. I believe that paragraph answers the concerns you expressed above, but please let me know if it doesn't. > [...]However I am certainly still open to suggestions - I think based on the other comments on this thread I will set up a github repository and people can see what I've done so far and comment and suggest as needed. See the remarks in README.developers concerning that possibility. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: David M. <da...@as...> - 2014-09-23 16:54:21
|
Hi, Alan, On Sep 22, 2014, at 12:37 PM, Alan W. Irwin wrote: > You are much more experienced with git than me. However, I thought > that rebasing a public branch was always a bad idea for the reasons I > mentioned concerning disappearing commits. I am positive a number of > resources I read when we were considering using a rebase workflow > mentioned this drawback to that approach. I agree that rebasing any part of a branch that has been pushed is a bad idea. It makes things complicated/inconvenient for those who are following the branch. > If they are right, then for rebase workflows like ours experimental > branches should not be made accessible on public git repos since the > bad choices afterwards are to rebase them periodically (which loses > prior public commits) to keep up with master development or else > become out of date with master. I can understand the desire for a nice clean linear history, especially since the code was just migrated from Subversion. Strict adherence to a rebase only workflow will definitely result in a nice clean linear history. Starting with a strict "rebase only workflow" requirement, the statement quoted above sounds reasonable, but the "experimental branches should not be made accessible on public git repos" part of it really sounds wrong to me. It makes me wonder whether a strict rebase only workflow is really best. > Which means some other alternative for > sharing experimental development topics must be used such as sending > patches to this list. My feeling is that working with such patches is > unlikely to be much of a burden since git has such good facilities > (format-patch and am) for generating and using such patches. Emailing patches is tedious and error prone. Yes, git has good facilities for this, but it has even better facilities for sharing changes: pushing to public repos. Asking developers to refrain from pushing their experimental branches to public repos is somewhat hobbling, IMHO. > Of course, if after we have gained some substantial experience with > the patch method for the special case of sharing experimental > development, we find that method is inhibiting our experimental > development, we should move to a merge workflow instead like suggested > by Brad King. But for now, let's give a good trial (at least 6 > months) to our current rebase workflow including its constraint (if > the resources I read are not overstating the case) that developers > should refrain from sharing their experimental branches via public > repos because it leads to bad further choices that typically come back > to haunt you with a rebase workflow. I am not familiar with Brad King's merge workflow, but I think allowing some merge commits can be a good thing. A compromise between a rebase-only workflow and an unconstrained merge workflow would be to allow merge commits on master but only if the first parent of such merge commits is also on the master branch. Developers would still be allowed (encouraged?) to follow a rebase-as-much-as-possible workflow, but they could also push their experimental branches to public repos without having to worry about rebasing them later. This would allow for a concise summary of changes on master by running "git log --oneline --first-parent master". Each non-merge commit on master would be shown along with the merge commit from each (non-rebased) topic branch, but the topic branch commits themselves would not be shown (because we passed "--first-parent"). Personally, I find merge commits to be a helpful way of partitioning the history of a project, especially when using gitk or "git log --graph". They are kind of analogous to subdirectories in that not having merge commits is kind of like keeping all your files in one directory. When running "git log --graph", merge commits make it easy to see which commits "go together" because they are shown on the same topic/experimental branch. Without merge commits, everything is just one lone thread of development. Of course it is possible to overuse merge commits, but generally if a topic branch is significant enough to share before rebasing it to master, then it's probably significant enough to warrant a merge commit on master when the time comes to bring the topic branch back into the fold. Hope this helps, Dave |
From: Alan W. I. <ir...@be...> - 2014-09-23 19:35:11
|
Hi David: I changed the subject line to something more appropriate. On 2014-09-23 09:54-0700 David MacMahon wrote: > Hi, Alan, > > On Sep 22, 2014, at 12:37 PM, Alan W. Irwin wrote: > >> You are much more experienced with git than me. However, I thought >> that rebasing a public branch was always a bad idea for the reasons I >> mentioned concerning disappearing commits. I am positive a number of >> resources I read when we were considering using a rebase workflow >> mentioned this drawback to that approach. > > I agree that rebasing any part of a branch that has been pushed is a bad idea. It makes things complicated/inconvenient for those who are following the branch. > >> If they are right, then for rebase workflows like ours experimental >> branches should not be made accessible on public git repos since the >> bad choices afterwards are to rebase them periodically (which loses >> prior public commits) to keep up with master development or else >> become out of date with master. > > I can understand the desire for a nice clean linear history, especially since the code was just migrated from Subversion. Strict adherence to a rebase only workflow will definitely result in a nice clean linear history. Starting with a strict "rebase only workflow" requirement, the statement quoted above sounds reasonable, but the "experimental branches should not be made accessible on public git repos" part of it really sounds wrong to me. It makes me wonder whether a strict rebase only workflow is really best. > >> Which means some other alternative for >> sharing experimental development topics must be used such as sending >> patches to this list. My feeling is that working with such patches is >> unlikely to be much of a burden since git has such good facilities >> (format-patch and am) for generating and using such patches. > > Emailing patches is tedious and error prone. Yes, git has good facilities for this, but it has even better facilities for sharing changes: pushing to public repos. Asking developers to refrain from pushing their experimental branches to public repos is somewhat hobbling, IMHO. I disagree. I have been applying user-generated patches to PLplot for years without any patch-related errors. And I really like the git format-patch command to generate a patch (which I have used to submit fixes to the cmake developer's list just like many others do there because that is the patch format particularly encouraged by Brad King for use by external developers). And I have had no problem already applying (using git am) an external patch for PLplot a git format-patch to PLplot supplied by an external developer. And sharing work using patches generated by git format-patch has not been an issue for years on the cmake-devel mailing list, the wine-devel list, and for the X intel graphics list and the cairo list when I was monitoring those. > >> Of course, if after we have gained some substantial experience with >> the patch method for the special case of sharing experimental >> development, we find that method is inhibiting our experimental >> development, we should move to a merge workflow instead like suggested >> by Brad King. But for now, let's give a good trial (at least 6 >> months) to our current rebase workflow including its constraint (if >> the resources I read are not overstating the case) that developers >> should refrain from sharing their experimental branches via public >> repos because it leads to bad further choices that typically come back >> to haunt you with a rebase workflow. > > I am not familiar with Brad King's merge workflow, but I think allowing some merge commits can be a good thing. A compromise between a rebase-only workflow and an unconstrained merge workflow would be to allow merge commits on master but only if the first parent of such merge commits is also on the master branch. That is exactly the "first-parent" merge workflow that is advocated by Brad King for experienced git users. (See <http://public.kitware.com/Wiki/Git/Workflow/Topic> for a full description.) > Developers would still be allowed (encouraged?) to follow a rebase-as-much-as-possible workflow, but they could also push their experimental branches to public repos without having to worry about rebasing them later. This would allow for a concise summary of changes on master by running "git log --oneline --first-parent master". Each non-merge commit on master would be shown along with the merge commit from each (non-rebased) topic branch, but the topic branch commits themselves would not be shown (because we passed "--first-parent"). > Personally, I find merge commits to be a helpful way of partitioning the history of a project, especially when using gitk or "git log --graph". They are kind of analogous to subdirectories in that not having merge commits is kind of like keeping all your files in one directory. When running "git log --graph", merge commits make it easy to see which commits "go together" because they are shown on the same topic/experimental branch. Without merge commits, everything is just one lone thread of development. Of course it is possible to overuse merge commits, but generally if a topic branch is significant enough to share before rebasing it to master, then it's probably significant enough to warrant a merge commit on master when the time comes to bring the topic branch back into the fold. You have made a number of points which I agree with (except for your comment about using patches). It is clear our adopted (enforced) git rebase workflow is only a baby step away from the subversion workflow which doesn't offer any alternative other than rebase. And such a baby step has the downside that extreme care should be used with publishing topic branches. (See the new wording in README.developers which expresses a consensus arrived at between Hazen and me concerning this issue.) But Brad King (who has advised many groups migrating from subversion over the years) strongly advised we take this baby step first to gain essential experience with git as a group, before moving to the (enforced) git merge workflow described above. > Hope this helps Yes it does because workflow choice is an extremely important topic to discuss for any group developing software using git. I think we are mostly in agreement here. Remember that most of our active developers (including me) were not familiar at all with using git for development. Therefore, the current rebase workflow advocated by Brad King for git beginners is right for us for now despite its drawback that caution must be used for published topic branches. But when in 6 months to a year or so from now we are ready to move from this current adopted rebase workflow to the merge workflow described above, I agree we will realize some additional benefits from git (i.e., less error-prone sharing of topics). However, note that we will need that substantial git group experience to use that merge workflow because of its downside. Which is that when pushes to the server are rejected because they do not maintain first-parent clarity, it is much harder to figure out the mess (according to Brad King) than for our current enforcement rule which simply rejects all merges that are not fast-forward. Also note that there is no simple way to enforce the first-parent workflow on the client side. In contrast, that can easily be done for the current rebase workflow (see our README.developers file for details) so that our developers have the opportunity to catch rebase workflow errors right away rather than having to wait to see whether the server rejects their push. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: David M. <da...@as...> - 2014-09-23 23:01:18
|
Hi, Alan, On Sep 23, 2014, at 12:35 PM, Alan W. Irwin wrote: > On 2014-09-23 09:54-0700 David MacMahon wrote: > >> Emailing patches is tedious and error prone. > > I disagree. I have been applying user-generated patches to PLplot for > years without any patch-related errors. And I really like the git > format-patch command to generate a patch (which I have used to submit > fixes to the cmake developer's list just like many others do there > because that is the patch format particularly encouraged by Brad King > for use by external developers). And I have had no problem already > applying (using git am) an external patch for PLplot a git > format-patch to PLplot supplied by an external developer. And sharing > work using patches generated by git format-patch has not been an issue > for years on the cmake-devel mailing list, the wine-devel list, and > for the X intel graphics list and the cairo list when I was monitoring > those. OK, maybe I my tolerance for handling patches is lower than others'. Git certainly does make it easy to generate and apply patches. It's almost the same a low-tech push/pull. Applying patches to the same commit from which they were generated will create a sequence of commits that have the same tree contents, log messages, and author info, but this sequence of commits will have different committer info and therefore they will be different commits (i.e. have different commit IDs). There are two issues here: 1) correctly applied patches will create different commits than the originals and 2) the patch files contain only the before/after SHA1 IDs of the patched files but no information about the un-patched files, which means that it is possible to successfully apply the patches to the incorrect commit. Issue 1 results in collaborators having different commit IDs for what are otherwise the same commit which can lead to confusion. Issue 2 results in collaborators having to carefully specify which commits their patches are based on (or more broadly applicable to) yet because of issue 1 they can't use commit IDs to do that. Yuck! In practice, this is rarely a problem for occasional/casual contributors, but for two actively collaborating developers sending patches back and forth it can be. >> Hope this helps > > Yes it does because workflow choice is an extremely important topic to > discuss for any group developing software using git. I think we are > mostly in agreement here. I'm happy to offer my thoughts where they are appreciated. I agree that we are mostly in agreement! :-) > note that > there is no simple way to enforce the first-parent workflow on the > client side. Push hooks can provide server-side enforcements, but I don't think git provides any way to enforce client-side workflows. > In contrast, that can easily be done for the current > rebase workflow (see our README.developers file for details) so that > our developers have the opportunity to catch rebase workflow errors > right away rather than having to wait to see whether the server > rejects their push. Just to nit pick: isn't this more of a pre-push check rather than client side enforcement? Nit picking aside, I agree that with rebase-only it easier to check for potential push problems on the client side *before* pushing. It sounds like you are on a prudent path forward! Dave |
From: Alan W. I. <ir...@be...> - 2014-09-24 05:41:17
|
On 2014-09-23 16:01-0700 David MacMahon wrote: > Just to nit pick: isn't this more of a pre-push check rather than client side enforcement? Nit picking aside, I agree that with rebase-only it easier to check for potential push problems on the client side *before* pushing. Hi David: Your comment above caught this nit-picker's attention. :-) What I was referring to (see README.developers) is we recommend each developer configure their local PLplot git repo using git config merge.ff only This command should stop merge commits of any kind for the local PLplot repo where it is run. I agree one of the client-side merges tested by this configuration occurs just before the push in our workflow so you could claim it is just a pre-push check. However, I think it is a little broader than that. For example, if the PLplot developer made some mistake by doing a git fetch and merge or equivalent git pull from some random unofficial private PLplot repository that included merge commits by accident, my understanding is this configuration option would catch that issue before the local repo was contaminated by those merge commits. > It sounds like you are on a prudent path forward! Thanks. So far so good. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |
From: David M. <da...@as...> - 2014-09-24 18:01:03
|
Hi, Alan, On Sep 23, 2014, at 10:41 PM, Alan W. Irwin wrote: > On 2014-09-23 16:01-0700 David MacMahon wrote: > >> Just to nit pick: [...] > > Your comment above caught this nit-picker's attention. :-) Takes one to know one! :-) :-) :-) > What I was referring to (see README.developers) is we recommend each > developer configure their local PLplot git repo using > > git config merge.ff only Maybe I'm just getting caught up on the semantics of the word "enforcement", but I don't consider that client side enforcement. It requires developer intervention to setup and can be overridden via the --no-ff option. If that counts as client side enforcement, then you could just as easily consider a client side pre-commit hook to be client side enforcement. That also requires developer intervention and has a command line override (--no-verify). > This command should stop merge commits of any kind for the local > PLplot repo where it is run. This prevents (in the absence of --no-ff) the local creation of merge commits, but it does not prevent merge commits from appearing in the history. > I agree one of the client-side merges > tested by this configuration occurs just before the push in our > workflow so you could claim it is just a pre-push check. However, I > think it is a little broader than that. For example, if the PLplot > developer made some mistake by doing a git fetch and merge or > equivalent git pull from some random unofficial private PLplot > repository that included merge commits by accident, my understanding > is this configuration option would catch that issue before the local > repo was contaminated by those merge commits. I think you are expecting too much from --ff-only. Running "get merge --ff-only new_branch" while on the master branch will prevent the creation of a merge commit if master cannot be fast-forwarded to new_branch. It will NOT prevent the fast forwarding of master to new_branch if any of the intervening commits are merge commits. git will happily fast forward master over any number of merge commits so long as master is an ancestor of new_branch. What could happen is that a PLplot developer starts with master that is up-to-date with sf.net and with "merge.ff=only" configured locally. Then the developer fetches a bunch of commits (that include merge commits) from a rogue repo. The rogue repo's build tests out OK, so the developer pushes the commits (including the merge commits) to the master branch on sf.net. None of this is prevented by --ff-only. With an suitable server side hook, the server side could reject the push, otherwise the public repo will end up with merge commits from the rogue repo. A client-side pre-commit hook could alert the developer to merge commits in the history, but that only fires if a commit is being done. A client-side pre-push hook could alert the developer to this situation before pushing, but that would happen only slightly sooner than a server-side pre-commit hook would reject things. Dave |
From: Alan W. I. <ir...@be...> - 2014-09-24 19:26:54
|
On 2014-09-24 11:00-0700 David MacMahon wrote: > Hi, Alan, > > On Sep 23, 2014, at 10:41 PM, Alan W. Irwin wrote: > >> On 2014-09-23 16:01-0700 David MacMahon wrote: >> >>> Just to nit pick: [...] >> >> Your comment above caught this nit-picker's attention. :-) > > Takes one to know one! :-) :-) :-) > >> What I was referring to (see README.developers) is we recommend each >> developer configure their local PLplot git repo using >> >> git config merge.ff only > > Maybe I'm just getting caught up on the semantics of the word "enforcement", but I don't consider that client side enforcement. It requires developer intervention to setup and can be overridden via the --no-ff option. If that counts as client side enforcement, then you could just as easily consider a client side pre-commit hook to be client side enforcement. That also requires developer intervention and has a command line override (--no-verify). > >> This command should stop merge commits of any kind for the local >> PLplot repo where it is run. > > This prevents (in the absence of --no-ff) the local creation of merge commits, but it does not prevent merge commits from appearing in the history. > >> I agree one of the client-side merges >> tested by this configuration occurs just before the push in our >> workflow so you could claim it is just a pre-push check. However, I >> think it is a little broader than that. For example, if the PLplot >> developer made some mistake by doing a git fetch and merge or >> equivalent git pull from some random unofficial private PLplot >> repository that included merge commits by accident, my understanding >> is this configuration option would catch that issue before the local >> repo was contaminated by those merge commits. > > I think you are expecting too much from --ff-only. Running "get merge --ff-only new_branch" while on the master branch will prevent the creation of a merge commit if master cannot be fast-forwarded to new_branch. It will NOT prevent the fast forwarding of master to new_branch if any of the intervening commits are merge commits. git will happily fast forward master over any number of merge commits so long as master is an ancestor of new_branch. > What could happen is that a PLplot developer starts with master that is up-to-date with sf.net and with "merge.ff=only" configured locally. Then the developer fetches a bunch of commits (that include merge commits) from a rogue repo. The rogue repo's build tests out OK, so the developer pushes the commits (including the merge commits) to the master branch on sf.net. None of this is prevented by --ff-only. With an suitable server side hook, the server side could reject the push, otherwise the public repo will end up with merge commits from the rogue repo. > A client-side pre-commit hook could alert the developer to merge commits in the history, but that only fires if a commit is being done. A client-side pre-push hook could alert the developer to this situation before pushing, but that would happen only slightly sooner than a server-side pre-commit hook would reject things. Thanks very much for these clarifications of the limitations of git merge --ff-only for the case where a PLplot developer uses a non-official repo where someone by accident has done a merge commit in violation of our workflow policy. Of course, PLplot development is (thank God) far from a police state. Instead, we expect all PLplot developers to be responsible about using non-official repos, i.e., use with extreme caution and especially pay close attention to the caveats concerning sharing work via non-official repos that are mentioned in README.developers. The only "enforcement" of our workflow policy that we do now is a suitable server side hook has been put in place to prevent merge commits being pushed and deletion of tags on the remote. See git_hooks/update. I have tested this hook with a test remote repo, and it appears to work as expected. For the others here, it is this server-side hook that is generating such messages as remote: Checking: commit 6f5dc7b546f05c79af9c6f406bd0c9db0f38736c on master not a merge commit (ok) remote: Checking: commit fce8ae116d4e0479d1ee9c5455b1198659262105 on master not a merge commit (ok) when you push your commits to our SF server repo. If you ever get a message that that test is failing, then you should carefully review the commits you are trying to push and figure out a way to get your desired changes on your local master branch without using merge commits before you push that local master branch to origin master. Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); the Time Ephemerides project (timeephem.sf.net); PLplot scientific plotting software package (plplot.sf.net); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ |