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

Clone repo instead of init + fetch and take advantage of git-autoshare #55

Merged
merged 1 commit into from
Sep 20, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions git_aggregator/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def query_remote_ref(self, remote, ref):
notably if ref if a commit sha (they can't be queried)
"""
out = self.log_call(['git', 'ls-remote', remote, ref],
cwd=self.cwd,
cwd=self.cwd if os.path.exists(self.cwd) else None,
callwith=subprocess.check_output).strip()
for sha, fullref in (line.split() for line in out.splitlines()):
if fullref == 'refs/heads/' + ref:
Expand Down Expand Up @@ -175,14 +175,14 @@ def aggregate(self):

is_new = not os.path.exists(target_dir)
if is_new:
self.init_repository(target_dir)
cloned = self.init_repository(target_dir)

self._switch_to_branch(self.target['branch'])
for r in self.remotes:
self._set_remote(**r)
self.fetch()
merges = self.merges
if not is_new:
if not is_new or cloned:
# reset to the first merge
origin = merges[0]
merges = merges[1:]
Expand All @@ -193,8 +193,44 @@ def aggregate(self):
logger.info('End aggregation of %s', self.cwd)

def init_repository(self, target_dir):
logger.info('Init empty git repository in %s', target_dir)
self.log_call(['git', 'init', target_dir])
"""Inits the local repository

If a remote is specified as a target, it will be cloned.
If the target has an associated branch, and that branch exists in the
remote repository, the clone will be limited to that branch.
If there is not a valid specified target, an empty repository will be
initialized.

:return: True if the repository was cloned
False otherwise
"""
repository = None
for remote in self.remotes:
if remote["name"] == self.target["remote"]:
repository = remote["url"]
break
branch = self.target["branch"]
# If no target is defined, init an empty repository
if not repository:
logger.info('Init empty git repository in %s', target_dir)
self.log_call(['git', 'init', target_dir])
return False
# If a target is defined, use it as the base repository
logger.info(
'Cloning git repository %s in %s',
repository,
target_dir,
)
cmd = ('git', 'clone')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmd = ('git', 'clone')
cmd = ('git', 'clone', '--filter=blob:none')

And maybe you don't even need git-autoshare.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the docs but don't understand. What does that do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yajo have a look at this article.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been doing tests and indeed we're gonna use it. It's a great feature. However, it's also true that it doesn't fix all the problems we want to fix, so we'll use it in combination with git-autoshare.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this suggestion is applied, IMHO the PR would be complete, as the rest of using git-autoshare is doable with the wrapper script and/or can be done in a later PR.

So could we move forward with this? Thanks.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only debatable thing. Should I add it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks.

# Try to clone target branch, if it exists
rtype, _sha = self.query_remote_ref(repository, branch)
if rtype in {'branch', 'tag'}:
cmd += ('-b', branch)
# Emtpy fetch options to use global default for 1st clone
cmd += self._fetch_options({})
cmd += (repository, target_dir)
self.log_call(cmd)
return True

def fetch(self):
basecmd = ("git", "fetch")
Expand Down