Skip to content
Snippets Groups Projects
Commit 41582380 authored by Georges Racinet's avatar Georges Racinet
Browse files

RefService.FindDefaultBranchName: filter out non existing target

It turns out that Gitaly returns an empty response on an empty
repository, whatever the value stored as default branch might be.
In case the default branch points to a branch that does not exist,
it returns the first branch it can find. This latter case cannot
happen with the state maintainer provided by py-heptapod.

This can be important for various mechanisms to actually set the
default branch. We're simply returning an empty response if the
default branch is set but its target does not exist. This amounts
to the same in all current scenarios. Later on we could return the
first GitLab branch if that becomes useful.
parent a08d999d
No related branches found
No related tags found
2 merge requests!204Making Heptapod 0.40 the new oldstable,!194CreateRepository with default branch
Pipeline #71323 passed with warnings
...@@ -124,7 +124,7 @@ ...@@ -124,7 +124,7 @@
repo = self.load_repo(request.repository, context) repo = self.load_repo(request.repository, context)
branch = get_default_gitlab_branch(repo) branch = get_default_gitlab_branch(repo)
if branch is None: if branch is None or gitlab_branch_head(repo, branch) is None:
# Very coincidental, but this ends up as empty GitLab branch name # Very coincidental, but this ends up as empty GitLab branch name
# which the chain of || in app/models/concerns/repository # which the chain of || in app/models/concerns/repository
# that eventually turns this in `nil`, which is the marker # that eventually turns this in `nil`, which is the marker
......
...@@ -38,6 +38,13 @@ ...@@ -38,6 +38,13 @@
request = FindDefaultBranchNameRequest(repository=grpc_repo) request = FindDefaultBranchNameRequest(repository=grpc_repo)
response = ref_stub.FindDefaultBranchName(request) response = ref_stub.FindDefaultBranchName(request)
# branch does not exist yet, hence gets filtered out
assert not response.name
wrapper.commit_file('foo', branch='other')
request = FindDefaultBranchNameRequest(repository=grpc_repo)
response = ref_stub.FindDefaultBranchName(request)
assert response.name == b'refs/heads/branch/other' assert response.name == b'refs/heads/branch/other'
grpc_repo.storage_name = 'unknown' grpc_repo.storage_name = 'unknown'
......
...@@ -324,6 +324,7 @@ ...@@ -324,6 +324,7 @@
if os.path.lexists(repo_path): if os.path.lexists(repo_path):
msg = ("creating repository: repository exists already") msg = ("creating repository: repository exists already")
corr_logger.error(msg)
context.set_details(msg) context.set_details(msg)
context.set_code(StatusCode.ALREADY_EXISTS) context.set_code(StatusCode.ALREADY_EXISTS)
raise RepositoryCreationError(repository) raise RepositoryCreationError(repository)
......
...@@ -109,6 +109,28 @@ ...@@ -109,6 +109,28 @@
.await??) .await??)
} }
pub async fn existing_default_gitlab_branch(
store_vfs: &Path,
) -> Result<Option<(Vec<u8>, Node)>, RefError> {
match get_gitlab_default_branch(store_vfs).await? {
None => {
info!("No GitLab default branch");
Ok(None)
}
Some(branch) => {
match lookup_typed_ref_as_node(stream_gitlab_branches(store_vfs).await?, &branch)
.await?
{
None => {
warn!("GitLab default branch not found in state file");
Ok(None)
}
Some(node) => Ok(Some((branch, node))),
}
}
}
}
/// From GitLab state files in `store_vfs`, resolve a given GitLab revision as a [`NodePrefix`] /// From GitLab state files in `store_vfs`, resolve a given GitLab revision as a [`NodePrefix`]
/// ///
/// Valid GitLab revisions include shortened commit SHAs, this is why the return type /// Valid GitLab revisions include shortened commit SHAs, this is why the return type
...@@ -120,5 +142,5 @@ ...@@ -120,5 +142,5 @@
revision: &[u8], revision: &[u8],
) -> Result<Option<NodePrefix>, RefError> { ) -> Result<Option<NodePrefix>, RefError> {
if revision == b"HEAD" { if revision == b"HEAD" {
match get_gitlab_default_branch(store_vfs).await? { match existing_default_gitlab_branch(store_vfs).await? {
None => { None => {
...@@ -124,4 +146,4 @@ ...@@ -124,4 +146,4 @@
None => { None => {
info!("No GitLab default branch"); info!("No GitLab default branch or pointing to missing branch");
return Ok(None); return Ok(None);
} }
...@@ -126,17 +148,7 @@ ...@@ -126,17 +148,7 @@
return Ok(None); return Ok(None);
} }
Some(branch) => { Some((_name, node)) => {
match lookup_typed_ref_as_node(stream_gitlab_branches(store_vfs).await?, &branch) return Ok(Some(node.into()));
.await?
{
None => {
warn!("GitLab default branch not found in state file");
return Ok(None);
}
Some(node) => {
return Ok(Some(node.into()));
}
}
} }
} }
} }
......
...@@ -16,8 +16,7 @@ ...@@ -16,8 +16,7 @@
RefExistsResponse, RefExistsResponse,
}; };
use crate::gitlab::gitlab_branch_ref; use crate::gitlab::gitlab_branch_ref;
use crate::gitlab::revision::{map_full_ref, RefError}; use crate::gitlab::revision::{existing_default_gitlab_branch, map_full_ref, RefError};
use crate::gitlab::state::get_gitlab_default_branch;
use crate::metadata::correlation_id; use crate::metadata::correlation_id;
use crate::repository::{default_repo_spec_error_status, repo_store_vfs}; use crate::repository::{default_repo_spec_error_status, repo_store_vfs};
...@@ -73,6 +72,6 @@ ...@@ -73,6 +72,6 @@
.map_err(default_repo_spec_error_status)?; .map_err(default_repo_spec_error_status)?;
Ok(Response::new(FindDefaultBranchNameResponse { Ok(Response::new(FindDefaultBranchNameResponse {
name: get_gitlab_default_branch(&store_vfs) name: existing_default_gitlab_branch(&store_vfs)
.await .await
.map_err(|e| { .map_err(|e| {
...@@ -77,4 +76,7 @@ ...@@ -77,4 +76,7 @@
.await .await
.map_err(|e| { .map_err(|e| {
Status::internal(format!("Error reading GitLab default branch: {:?}", e)) Status::internal(format!(
"Error reading or checking GitLab default branch: {:?}",
e
))
})? })?
...@@ -80,5 +82,5 @@ ...@@ -80,5 +82,5 @@
})? })?
.map(|ref name| gitlab_branch_ref(name)) .map(|ref name_node| gitlab_branch_ref(&name_node.0))
.unwrap_or_else(Vec::new), .unwrap_or_else(Vec::new),
})) }))
} }
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
import pytest import pytest
import time import time
import py.path
from hgext3rd.heptapod.branch import set_default_gitlab_branch
from hgext3rd.heptapod.special_ref import ( from hgext3rd.heptapod.special_ref import (
special_refs, special_refs,
) )
...@@ -91,6 +93,41 @@ ...@@ -91,6 +93,41 @@
@parametrize('hg_server', ('hgitaly', 'rhgitaly')) @parametrize('hg_server', ('hgitaly', 'rhgitaly'))
def test_compare_find_default_branch_name_fallbacks(
gitaly_rhgitaly_comparison,
hg_server
):
fixture = gitaly_rhgitaly_comparison
git_repo = fixture.git_repo
rpc_helper = fixture.rpc_helper(
hg_server=hg_server,
stub_cls=RefServiceStub,
method_name='FindDefaultBranchName',
request_cls=FindDefaultBranchNameRequest,
)
# empty repo, but with default branch set
branch_name = b'branch/maybe_later'
set_default_gitlab_branch(fixture.hg_repo_wrapper.repo, branch_name)
# git_repo.set_symref needs a `py.path` path, not a `pathlib.Path`
git_repo = fixture.git_repo
git_repo.path = py.path.local(git_repo.path)
git_repo.set_symref('HEAD', 'refs/heads/' + branch_name.decode('ascii'))
rpc_helper.assert_compare()
# non-empty repo, Gitaly will return the first existing branch it finds
# (actually works because mirroring/state_maintainer fixes the default
# branch to be an existing one)
fixture.hg_repo_wrapper.write_commit('foo', message="Some foo")
gl_branch = b'branch/default'
# mirror worked
assert git_repo.branch_titles() == {gl_branch: b"Some foo"}
rpc_helper.assert_compare()
@parametrize('hg_server', ('hgitaly', 'rhgitaly'))
def test_compare_ref_exists(gitaly_rhgitaly_comparison, hg_server): def test_compare_ref_exists(gitaly_rhgitaly_comparison, hg_server):
fixture = gitaly_rhgitaly_comparison fixture = gitaly_rhgitaly_comparison
wrapper = fixture.hg_repo_wrapper wrapper = fixture.hg_repo_wrapper
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment