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

Missing error message: when cloning a repository with main branch not being master #345

Open
cgestes opened this issue Apr 7, 2022 · 7 comments

Comments

@cgestes
Copy link
Contributor

cgestes commented Apr 7, 2022

With debug activated:

=> Cloning missing repos
/home/cedric/src/jots $ git clone --origin origin [email protected]:jotshq/gotrue-dart.git --branch master --recurse-submodules gotrue-dart                                                      
Error: Failed to clone the following repos                                                                                                                                                    
* gotrue-dart : Cloning failed

the main branch of the repository is main not master. (this is the default in github now)

(btw I had the same issue with the manifest)

@cgestes
Copy link
Contributor Author

cgestes commented Apr 7, 2022

maybe we could make it work, or provide an appropriate error message?

Also it would make sense to have a verbose output when it fails (or a hint that it exists), otherwise I need to re-run the command with verbose.

@Lecrapouille
Copy link
Contributor

@cgestes main branch is a GitHub specific branch name (not Git specific) therefore you shall indicate it as name branch like any dev branch.

@cgestes
Copy link
Contributor Author

cgestes commented Apr 12, 2022

you are right Main is github specific.
I think we can use the default cloning branch of git. (Git doesnt fail if no branch is specified)
We probably can use the default branch of the repository instead of failing 😁

@dmerejkowsky
Copy link
Collaborator

dmerejkowsky commented Apr 23, 2022

Here's what I think is happening.

I assume your manifest looks like this:

repos:
  - dest: gotrue-dart
    url: [email protected]/jotshq/gotrue-dart

with no branch specified.

when we parse the manifest, we go through the following code

class Manifest:
   ...

   def _handle_repo(self, repo_config: Any) -> None:
       dest = repo_config["dest"]
       branch = repo_config.get("branch", "master")   # 'master' hard-coded there
       tag = repo_config.get("tag")
       ....

This explains why tsrc sync runs git clone with --branch master.

We could just modify the code to look like this:

   def _handle_repo(self, repo_config: Any) -> None:
-     branch = repo_config.get("branch", "master")   
+     branch = repo_config.get("branch")
       
class Repo:
    dest: str
-   branch: str
+   branch: Optional[str]

But then tsrc fails apart. tsrc sync (and other commands) need to know what the current branch of the repo is.

The only clean fix I can think of is to require the branch to be specified in the manifest, but this is a breaking change.

@dmerejkowsky
Copy link
Collaborator

Writing a failing test for this was a bit harder than I expected, but you can see it at the corresponding branch

@cgestes
Copy link
Contributor Author

cgestes commented Sep 12, 2022

Can we get a proper error message when the branch does'nt exist?

Got this bug again, lost 10min again figuring out... even the verbose option doesn't show what is happening. There is really no easy way to understand what's happening.

Btw I think git is going to change the name of the default branch soon also.

@cgestes cgestes changed the title Fail when cloning a repository with main branch not being master Missing error message: when cloning a repository with main branch not being master Sep 12, 2022
@dmerejkowsky
Copy link
Collaborator

Can we get a proper error message when the branch does'nt exist?

Yup, we can. Turns out we introduced a generic run_git() method in the Task class but forgot to take advantage of it in the Cloner class ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants