-
Notifications
You must be signed in to change notification settings - Fork 277
[release/0.4.0] "git subrepo push" always applies the -u option? #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Additional info: if I manually restore the original remote and branch in the |
I am thinking a little about what the expected behavior should be. Right now it seems that the .gitrepo file is always updated with new commit info, if you want to update the branch/remote you need to add the -u option. For my own workflow, I would say that I prefer having the .gitrepo file update at all times, maybe introducing a flag that prevents update. So if I want to push stuff to another branch/repo nothing will happen locally. What do you think about that? |
I may be wrong, I'm definitely new to
If I am correct, Unless, obviously, the docs need to be updated and all commands are actually working the same way in If we want to restore the original behavior, maybe there's a way to specify the |
First: @plach79, good comments! For 0.3.1 the difference is that you don't rely on the information in the .gitrepo, instead you parse the tree to find the information. So in that case you don't have a default behavior of updating the .gitrepo file. It was only done with -u. In the 0.4.0 subrepo structure is based on the information in the .gitrepo file, parent show when something was last pushed and commit is used as a soft pointer to connect main repo commits with the sub repo commits. So the default behavior is to update .gitrepo when you pull/push. Question is what actually happens to consistency in the different cases:
@ingydotnet What do you say? If I remember correctly you have been an advocate for having "no additional commit" on push as the default? My personal taste is that I want to be able to have a "push-push" workflow, allowing me to keep on working without any additional operations. |
Makes sense, I'll just outline my workflow here to clarify why I'd need not to update the I have a private github setup with two main projects that are sharing a set of libraries. We would like to be able to work on the libraries from both projects, but still be able to keep them in-sync without having to manually copy over changes. Since the two projects are using the PR workflow we would like to be able to do the same with the libraries subrepo(s). This is how we are currently planning to work:
|
I understand that a PR workflow is based on this circular approach: Would it be possible to change that so that you instead have the subrepo fork as your pull/push from the main repo? Then you can have a gatekeeper review the changes in the subrepo fork and work against the subrepo when you want to share them with other projects? Why? because one of the main features in subrepo is that we need to keep track on where we are relative to the subrepo, using the parent/commit data in .gitrepo will accomplish this. If you use the circular approach I think there will be conflicts along the way. If you try the more linear approach, I think it will be much easier to handle changes. And I can't see that it adds in complexity, you still need to handle actual subrepo changes in it's own fork. @plach79 , What do you think? |
I'm not sure I understand your suggestion, sorry, can you provide an example?
We would like to be able to work on the libraries from the project repo because these are exposed as plugins and so are hard to test in isolation (only unit testing, no integration testing available). |
Example:
Another thought if you want to verify the subrepo in the context of main repo:
Was that any helpful? |
Thanks, @grimmySwe, this was helpful! Suggestion 1 wouldn't work for us because each developer has a different subrepo fork. We could use a remote alias to clone the subrepo and have everyone configure their own fork url with the same remote alias, but this would just reverse the problem. When the GK pulls changes from the upstream repo, Suggestion 2 was exactly the other option I was considering, the only problem with it would be that the discussion around the library issue won't be in the subrepo, but scattered through out the project repos. However this looks more viable to me. |
@grimmySwe, just to be sure, we are just waiting for @ingydotnet's feedback on #313 (comment), right? |
@grimmySwe, I'm reviewing the release/0.4.0 branch and just looking through issues. Looking briefly into this one and then working with #318 earlier, I am now The 0.3.1 release and the master branch don't update gitrepo for a pull, and I Can you explain why and when it got added into the release/0.4.0 branch? In my mind, a parent repo shouldn't care when some other repo was updated. |
FWIW, I think it's important to be explicit when you want to update the parent/commit because you could do a subrepo push that updates the .gitrepo file, and then never push that new commit. |
In regular git, you can do a push-push workflow. As long as no one else changes a branch you don't need to pull. Every time you push in regular git, the origin reference is moved to a new location to remember the state. In git subrepo the 0.4.0 design is based on the .gitrepo, we use "commit" to know the latest commit in the subrepo and "parent" as reminder to how much we already pushed. So in a sense this is the reference that moves for each time you push. Unfortunately you can't do this without actually making a local commit with a new .gitrepo. If we don't update the .gitrepo file, we have no track of what is actually done and that will force us to perform a pull before we do our next push. And we can't differentiate our own commits form any other commits so we will have to go through the entire process of recreating the history from the previous .gitrepo information. The odd use cases are when you want to push things into new branches/remotes. Then you might want to avoid updating the .gitrepo file as it will turn everything into a mess unless you update the entire .gitrepo file. Worst case is that you only update commit/parent but you actually pushed into another branch/remote, then you will end up in trouble when you continue on your previous branch/remote. So there are maybe three different use cases:
In both 1 and 3, I see that you update the .gitrepo, for 2 you should not update the .gitrepo.
Yes, that is true, but if we don't update the .gitrepo file we don't have any trace on what we have done. There will be a discrepancy between the state of the repo and the subrepo. If I were to clone the repo I would get the latest state of the subrepo but my .gitrepo file would still point at a previous state. For me it's just a help that I get an extra commit directly, then I just need to push the parent repo. If I don't get that, I would have to pull the subrepo and then push the update .gitrepo. |
From my own workflow, I don't want git-subrepo to read the tree and try to find an ancestor So I would like to be able to force it to the use the .gitrepo file as reference for commit. |
@FanchTheSystem So you do like the extra commit updating the .gitrepo? |
Yes I like it :) |
This discussion is definitely above my head, I can only say that if the current behavior is going to be the new normal, there should be a way to opt-out from it. I've ben testing #314 with the workflow described in #313 (comment) and it has been working flawlessly so far. |
@plach79 thanks for your input! Even if you think the discussion is above your head, your input is extremely valuable. |
It seems as though adding the commit is just a helper for git-subrepo to do My stance here is that we make adding this commit something that is opt-in. We I have always expected that subrepo would do a pull before a push. I also have Adding a commit when no content was changed locally feels bad to me. I wouldn't This gets worse in the case of #318, where I'm assuming here that if we don't have the information, we can still figure it Thoughts? |
@grimmySwe I'm not entirely certain what you mean by push-push workflow. Whatever it means, git is certainly not adding commits when this info is updated, right? Maybe you want to add a subrepo last push reference here? |
Starting with the easy part:
I totally agree that 19 commits is way to much, there must be a better way (as the one you suggested in that issue) Personally I think that the .gitrepo commit created by push saves me time, there will be a commit anyway by the pull that I need to do before the next push, so for me I can't see that there is a real difference. When thinking about it, I actually prefer the push commit as it tells me that I do something, if it was always pull commits, then I don't know if it was something I did or just a regular update. Comparing it with regular git, I wouldn't want to pull in my own changes each time I push.
With the current design I would think that it's more logical to have the commit created by default and have an opt-out option that you can use in the cases when you don't want it. I understand that it is opposite to previous versions, but I think that it will generate less support. |
I respect that and I'm here to help! :) What I am suggesting is that you can get this behavior in several ways:
That's not actually true. There will be a new commit if something changed in There are situations (like subrepo iteself for instance) where the changes to
2 things here:
So in one sense, I could come to the sane conclusion that there are never push @grimmySwe can you find the commit that started adding the commit on push? I'll do some more thinking on this and report back later today. |
Does it means .gitrepo file may never be updated ? So If I use it only for push, and never use update. I may be wrong, I don't know well how git subrepo use the .gitrepo file |
The gitrepo file will be updated on a pull or on a We also might want to detect a conflict resolved by hand and tell people they should do an update or actually prompt them to do an update. The thing we are missing here is real test situations on public repos. THat's a shame because these situations are easy to recreate even if they happen a year ago. If anyone can point me to public repos and commits that cause various conflicts it would be much appreciated. I'm also going to write some brute force scripts to find some of these today. Can anyone think of a good way to search github to find public subrepo usage? I just tried but with little success. I'll report back later today with my findings. |
I have a real situation here: Our Platform where created without subrepo and commit where done on both side Due to this special situation i have had some conflict, and I was unable to push well with git-subrepo 0.3, using the current 0.4 branch solve my issue, and now I am able to push. I think it is because 0.4 |
@FanchTheSystem thanks. I'll play around with this. I was just thinking... If people are having problems with their private repos and they trust me to look at them, you can let me know and I will create a private repo called |
I think we don't need private repo for this. All our work are with GPL or LGPL Licence, you can fork as you like ;) Also to make good real test, it need 21 repository, one for main repo and one for each subrepo :) But It is easy for me to create a full copy of our Platform on any github account where I have full access (I have some script for this) So maybe I can create this as public on my own account and give you access. (It will do it tomorow, if you confirm you are ok with this) |
@ingydotnet So I took a good night sleep and thought about this some more. I can't help it, but I really think that push should update .gitrepo and create a new commit as the real default. Why? In subrepo 0.4.0 the .gitrepo:commit attribute is used as a virtual reference between commits. So when it creates the internal branch structure it will detect the correct state and make it in simple way. If we leave this out, it will have to redo everything AND most likely you will need to reapply changes causing conflicts along the way. I would compare the .gitrepo file with the "origin/ref". I do understand that there are cases where you might want to avoid creating this commit but that should only be relevant if you use the -r/-b flags, pushing to another branch/remote. In "all" other cases that I can think of I really want the .gitrepo to reflect the current state, not a previous state. Having subrepo do pull before each push, would not mitigate this as you will still end up where your .gitrepo file doesn't reflect the current connection between subrepo and parent repo. I also get back to the regular git comparison, for me I think it's important that subrepo tries to be as close to git as possible. Yes there are some differences, but the standard push/pull workflow should be as similar as possible. Ok, regular git push doesn't create an actual commit BUT it does update your local repo so it is not a read only operation. Finally I truly believe that it's much easier for a new subrepo user to understand the flow of how things work if the update is there from the start. Yes, there could be ways to disable it, but for me that is super user actions, as afterwards you really need to know what you are doing. Version 0.2.X used .gitrepo, but for me it had some shortcomings in that it didn't allow you to work push-push and it could potentially lose some changes along the way. With all that said, for me personally, I have no problem with an option because I know how it works. My biggest concern is that without having the .gitrepo update automatically on pushes, there will be an endless stream of questions from new subrepo users trying to understand why their pull operations caused conflicts. |
Regarding the -r/-b options there should be some more thought there:
Note that you should never only update the remote and branch in the .gitrepo file, then your commit/parent attributes will most likely be invalid. So I see three use cases: Question is what the default behavior should be, depending on the conclusion of the push .gitrepo commit here, I guess we should align this with that. |
@grimmySwe thanks for the input. My executive stance at this point in time is:
I have a feeling that we can make the default behavior more nuanced; ie have it know when it is appropriate to add a commit, or at least tell the user they should Adding a push commit by default throws conflicts with my goal that subrepo stays almost completely out of sight except when absolutely necessary, and that's something I don't want to erode. Hopefully today I'll have time to make some repeatable situations and put them in a place we can discuss. |
@grimmySwe 2 questions:
Cheers. |
That is me describing simple work where I only push. No one else is working on the subrepo/branch, so I just implement things and push. For me that is a common use case. At work we have a great CI system so we encourage to push often and get early feedback.
It is in f98012a, reason is that I use |
I totally agree that subrepo should stay out of sight if possible. And it would be great if we don't need the push commits. I'll see if I can test some alternatives and see if there is a way around it. |
Made some simple tests, first disabling the push commit. If I didn't do anything between the subrepo push and the subrepo pull, it worked without conflict, if I did anything related locally first, before the subrepo pull it often ended up with a conflict. I guess that the reason for that is that the merge behind the scene is a three point merge, tip of subrepo, tip of local subrepo and then the common ancestor. So we don't actually know/care that the tip of the subrepo is actually part of our local history. |
@grimmySwe Sorry, I had a big family day yesterday and today is a travel day, but I like where things are going. Let's dig in and keep working through this. |
I thought of an idea for this. We can make it the default to add a .gitrepo commit after a push that involves a conflict resolution. The last step of a conflict resolution is: We can make that form add a commit by default, because that's when it is probably really needed. You can still add the commit with Thoughts? PS Tomorrow I'll spend some time on 0.4.0 release planning. I'd like to spend at least one hour a day on this until we can get it out. |
It will prevent some conflict due to the fact you update .gitrepo and that is always good. But the large source of conflicts are what you do after you pushed, so I don't see it as a fool proof solution. From a debugging perspective it might even be a little harder to have different behaviors. Then it's harder to explain why things happen "sometimes". I have tried to come up with something to improve the situation, but I have failed so far. The main culprit as I see it, is that the merge is a three point operation, and if you don't use the right three points you end up with a conflict. Worst thing by not updating the .gitrepo file, is that it is easy to get conflict even if I am the only one working on the subrepo. Why? Because I don't record the changes locally so every time I push things to the subrepo it will be as anyone has made changes. |
Uh oh!
There was an error while loading. Please reload this page.
I am not sure whether I found a bug or this is intended, but on the
release/0.4.0
branch (ba19aa7), if I try to push while specifying an alternate remote and branch (I'm trying to implement a pull-request workflow for the subrepo),.gitrepo
is updated with those remote and branch values, as if I specified the-u
option.This is not what I would expect, in fact I'd assume that, unless
-u
is specified, the original remote and branch are kept. Am I missing anything?The text was updated successfully, but these errors were encountered: