History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: CIB-2024
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: 3 3
Assignee: jason
Reporter: Rohan McGovern
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Pulse

When using git, Pulse server creates merge commits under some conditions

Created: 30/Jun/09 01:35 AM   Updated: 02/Jul/09 09:48 AM
Component/s: Scm
Affects Version/s: 2.0.32
Fix Version/s: 2.1.4, 2.0.33

Original Estimate: Unknown Remaining Estimate: Unknown Time Spent: Unknown


 Description  « Hide
In one of our projects, I saw a change pop up by the author "Pulse Server", with description "Merge branch ... into local".

The branch in question had been rebased so that a simple "git pull" resulted in a merge which wasn't a fast-forward. Once this happens, all future changes will also be merges by "Pulse Server", meaning the revisions tested by Pulse never match anything in the actual SCM server.

Suggest using only `git fetch' and not having any local branch? (though rebasing still won't work right due to CIB-1874 .)

 All   Comments   Work Log   Change History      Sort Order:
jason - 30/Jun/09 06:35 AM
Hi Rohan,

Thanks for the report. I'm not sure that just fetching will work out as some of the master functionality uses the local checkout (e.g. browsing the files). I'm also not sure (I will need to try it) that there won't be problems anyway even if we just fetch. It sounds like you are rebasing a public branch, effectively changing the history of the branch? Generally speaking, this is likely to cause problems if Pulse (or anything else working off the branch) has already seen the commits which are subsequently removed. For example, if Pulse already did a build at one of those commits, then it may later ask for more details around that commit (e.g. changes since that commit) - but the commit will no longer exist. For reasons similar to this I think most workflows discourage rebasing of shared branches?

Rohan McGovern - 30/Jun/09 07:57 PM
Yes, this was changing the history of a branch which is visible to Pulse.

We do in theory disallow changing the history of a "public" branch but there's some debate internally over exactly when a branch is considered "public". Some developers want to maintain their own repos, which no-one but themselves are expected to commit to, which they will rebase (or otherwise change the history of) at will until it's time to merge with one of the mainline branches.

I predict that attempting to forbid these developers from using this workflow for the sake of Pulse would encounter significant resistance :-) Also, I can't tell them to use the "personal build" feature since that doesn't (yet) work with git. So I don't currently know of a way to test these repos with Pulse in a way which doesn't run into this issue.

The best case scenario would be if Pulse supported all git workflows, but I understand that may be impractical if the SCM code was designed around the working of more traditional SCM systems. However I'm sure the situation can be improved somehow from the current behavior which is to more or less silently start testing the wrong thing.

It should be easy to detect when this kind of history modification has occurred, e.g.:

git fetch origin
git merge-base <last_sha1> origin/master

If the output of merge-base is last_sha1, then we're OK, it's a straight fast-forward from last_sha1 to the new HEAD; in any other case we're screwed.

Also, for your specific example of commits going missing: if history is modified, the commits will still exist locally, commands like `git show old_sha1' still work. They'll only be destroyed if they're not reachable from any branch/tag and git is allowed to do a garbage collection. So, there are ways to solve this problem, e.g. when a history cut is detected, create a new branch locally pointing at the "old" history, then the old commits will stick around forever (unless a "reinitialise" of the project is done).

jason - 01/Jul/09 04:32 AM
Hi Rohan,

Thanks for the extra details. I had not considered the case where the repository is only public in the sense that it has been seen by Pulse to effectively run personal builds from a developer's own repository. As it happens we would like to support this use case as best we can (with git being so flexible we want multiple ways of doing personal builds even when we have more built-in support for it).

I think the problem is not so much that we've only supported traditional SCMs until now, but more that rewriting history makes it difficult for Pulse to maintain its own clone in a clean state. Things tend to get confusing quite quickly when history is rewritten, so much so that it is difficult to find information about how to handle it in general. And some of the operations Pulse wants to do for you (e.g. show new changes in a build) no longer have a straightforward answer if history has been changed between builds. So I think if we can/do support this kind of rebasing, it may have to come with some loss of functionality - hopefully just for the next build after a rebase. For example I can imagine one solution of just detecting this case and reinitialising our internal clone when it happens, to make sure that it doesn't build up potentially confusing cruft.

Because we cache the commit data, losing commits we have seen before only really affects our calculation of what has happened "since" a revision - e.g. in polling and calculating changes between builds. So if there is a willingness to forgo seeing changes between builds when there has been a rebase, I think we can safely reinitialise.

jason - 01/Jul/09 01:09 PM
I have a possible resolution for this, along the lines described. Using the idea you mentioned of going a fetch and checking HEAD vs the merge base, it seems I can detect non-fast-forward merges. In this case an error is thrown, to avoid the possibility of Pulse creating merge commits. The error indicates that a reinitialisation is required to get out of this state. To avoid needing to do this manually, if the SCM polling detects this type of error it will trigger reinitialisation. Note that for this to work, monitoring must be turned on for the SCM -- but you do not need to have an SCM trigger configured. It is possible for other operations to hit this error, but as they are spread about I have not attempted (so far) to detect this special case in those places, so they will just report the error without tripping a reinitialisation.

As part of this change I also added the same detection to bootstrapping updates (for the clean/incremental update checkout schemes). When those bootstraps detect a non-fast-forward merge a clean checkout will be forced.

I hope this works in practice for you. I have tested with a couple of types of history rewriting here, and it does the trick, and as long as you have monitoring on it should be automatic most of the time. In some cases a manual reinitialisation may be required. If this happens regularly, please let us know so we can make it smarter about reinitialising.

Refer to change 6082.

jason - 02/Jul/09 09:48 AM
Merged to trunk in change 6087.