Skip to content
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

Make merge working for all remote branches (not only master) #16430

Closed
wants to merge 1 commit into from

Conversation

paulslaby
Copy link
Contributor

Git repository had master branch hard-coded. I replaced it with actualy checked-out branch.

Links

Steps for Testing/QA

If you create a repository with no master branch and want to import it to automate, it failed. Now it is fixed. More information in Bugzilla

@mkanoor
Copy link
Contributor

mkanoor commented Nov 13, 2017

@paulslaby In the lib/git_worktree there are multiple references to master

MASTER_REF = 'refs/heads/master'

Won't all of these have to be fixed.

I am wondering if there is a bug in

repo.send(:fetch_and_merge)

Shouldn't this be calling just fetch, since we are only working with bare repos, and the user is not allowed to change the bare repo. So I am not sure why we would be merging anything

@paulslaby
Copy link
Contributor Author

@mkanoor Thanks for the reply. In git ecosystem fetch only refreshes information about remotes. We need to do something like git pull, and rugged does that through fetch_and_merge. You are right, that pull method is more descriptive than fetch_and_merge.
So we only need to tell it, which remote and branch we want to merge from. According to libgit2/rugged#314 this is the best way to get branch name.

You are right, that I could remove MASTER_REF completely, so I will, do it.

@paulslaby paulslaby changed the title Make merge working for all remote branches (not only master) WIP: Make merge working for all remote branches (not only master) Nov 14, 2017
@chessbyte chessbyte changed the title WIP: Make merge working for all remote branches (not only master) [WIP] Make merge working for all remote branches (not only master) Nov 14, 2017
@miq-bot miq-bot added the wip label Nov 14, 2017
@paulslaby paulslaby changed the title [WIP] Make merge working for all remote branches (not only master) Make merge working for all remote branches (not only master) Nov 16, 2017
@paulslaby
Copy link
Contributor Author

@mkanoor Hello again, I removed all hard-coded references to master. What do you think now?

I wanted to write tests, but I do not know, how to mock remote repository.

@miq-bot miq-bot removed the wip label Nov 16, 2017
@mkanoor
Copy link
Contributor

mkanoor commented Nov 21, 2017

@paulslaby The lib/gitworktree was part of a project related to implementing Automate models in a Git database. In that model the master branch was the only branch that was being allowed. So I would not remove the MASTER references completely.
The repos that we have currently for Automate are not editable, so there is nothing to merge.
So we should just fixing the call to use fetch.

repo.send(:fetch_and_merge)

Change that to fetch and it will solve your problem.

@paulslaby
Copy link
Contributor Author

paulslaby commented Dec 13, 2017

@mkanoor Thank you for you tip. Unfortunately, it does not work.
What will we do about this issue? I have a working patch and I'd like to help you to get it working for others too.

@mkanoor
Copy link
Contributor

mkanoor commented Dec 13, 2017

@paulslaby I have been trying to add tests but the specs were getting stuck. So I need to figure that out first.

@mkanoor
Copy link
Contributor

mkanoor commented Dec 13, 2017

@paulslaby Have you looked at the Travis failures.

@miq-bot
Copy link
Member

miq-bot commented Dec 14, 2017

Checked commit paulslaby@0c55372 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@paulslaby
Copy link
Contributor Author

@mkanoor Travis is timing out from unknown reason for me. Have you faced it already?

@mkanoor
Copy link
Contributor

mkanoor commented Dec 19, 2017

@paulslaby
We have a locking mechanism in the lib/git_worktree that was broken with you changes so I have fixed that and added specs. In PR #16690

@paulslaby
Copy link
Contributor Author

@mkanoor Thank you. I am closing this one as your one will resolve it.

@paulslaby paulslaby closed this Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants