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

[bug] Git.checkout() with a target folder doesn't work with the remaining Git functions #14058

Closed
fschoenm opened this issue Jun 9, 2023 · 7 comments · Fixed by #14063
Closed
Assignees
Milestone

Comments

@fschoenm
Copy link

fschoenm commented Jun 9, 2023

Environment details

  • Operating System+version: n/a
  • Compiler+version: n/a
  • Conan version: 2.0.6
  • Python version: 3.11

Steps to reproduce

The conan.tools.scm.Git interface seems lacking. If I specify a target folder in the clone() command, there seems to be no straight-forward way to correctly use the other Git functions like checkout(). They're all going to be executed in the wrong directory.

    def source(self):
        git = Git(self)
        git.clone(url=self.scm["url"], target=os.path.join(self.source_folder, "src"))
        git.checkout(commit=self.scm["revision"])

Logs

Giving this error:

        git.checkout(commit=self.scm["revision"])
        ConanException: Command 'git checkout 42353e2' failed with errorcode '128'
b'fatal: not a git repository (or any parent up to mount point /)\nStopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).\n'
@memsharded
Copy link
Member

Hi @fschoenm

Thanks for your report.

The Git() constructor itself is nothing but a caching of the target folder to use, for convenience, and the folder is a public attribute. So this works:

 def source(self):
        git = Git(self)
        target = os.path.join(self.source_folder, "target")
        git.clone(url="{url}", target=target)
        git.folder = target
        git.checkout(commit="{commit}")
```

I'd say this is not a bug, but intended. I will submit a PR to propose to make this automatic, but probably hiding the detail is not that great, and having explicit the fact that the current folder for "git" commands has changed is better, lets see what the team thinks.

@fschoenm
Copy link
Author

The Git() constructor itself is nothing but a caching of the target folder to use, for convenience, and the folder is a public attribute. So this works:

Actually, I didn't really understand why there are two folders involved, a target folder and a working directory, and what is their relationship. Do you mean it might work to just set the folder and all commands (including clone() and checkout()) should work? Then I really don't understand what the target is for.

@memsharded
Copy link
Member

memsharded commented Jun 11, 2023

The folder that is defined is the cwd where the git command will be executed, no more no less. But when git clone is called, there can be 2 folders, the cwd and the target folder. If more than 1 repo is cloned (not typical, but sometimes happen) there will be 3 folders, one cwd and 2 target folders.

For the use case at and, it is also possible (and probably what makes sense) to do:

 def source(self):
      git = Git(self, "target")
      git.clone(url="{url}", target=".")
      git.checkout(commit="{commit}")

Note the target="." clones in the current folder, not creating another extra subfolder

@memsharded memsharded added this to the 2.0.7 milestone Jun 14, 2023
@memsharded
Copy link
Member

Closed by the test in #14063.
Will add some more documentation, clarifying that for this use case:

    git = Git(self)
    git.clone(url="{url}", target="target") # git clone url target
    git.folder = "target"                   # cd target
    git.checkout(commit="{commit}")         # git checkout commit

@fschoenm
Copy link
Author

@memsharded Sorry but I don't really understand the use case. The multiple directories are kind of confusing and I think if I want to only clone and checkout a commit, I shouldn't have to handle two directories.

How does that even work with your imaginary use case of having multiple clones from the same Git object? Aren't you going to run into exactly the same problem, that the checkout() call does not work on the correct directory? Or are you supposed to set the git.folder property every time before you want to work on a different Git clone?

The only use case that seems straight forward is having folder not set and using target="." (which doesn't work well with #14056 btw). Why not have a wrapper for a Git repo instead of a wrapper for the git tool? All the other functionality of Git() works is supposed to work on only one repo anyway.

@memsharded
Copy link
Member

Sorry but I don't really understand the use case. The multiple directories are kind of confusing and I think if I want to only clone and checkout a commit, I shouldn't have to handle two directories.

If you want to clone and checkout a commit you do have to handle 2 directories:

$ pwd 
/home/user/myfolder
$ git clone url-of-repo/repo.git
$ cd repo  # <= This is necessary to work
$ pwd
/home/user/myfolder/repo
$ git checkout tag/commit

The cd is necessary, it is mandatory to handle 2 directories. I am not sure where is the confusion.

How does that even work with your imaginary use case of having multiple clones from the same Git object?

It is not imaginary. I said it clearly "(not typical, but sometimes happen)". I am always happy to help, but I hope you are not suggesting I am making things up, that would be crossing a line. I have seen it with my eyes, in videocalls, in more than one user codebase, fetching sources from one repo with one git.clone(), and fetching other stuff (aux scripts, patches, etc) from another repo, doing another git.clone(). It works by changing the git.folder, yes, in exactly the same way you would cd on the command line.

git = Git(self)
git.clone(url="{url-sources}", target="sources") # git clone url-sources sources
git.clone(url="{url-scripts}", target="scripts") # git clone url-scripts scripts
git.folder = "sources"                           # cd sources
git.checkout(commit="{commit-sources}")          # git checkout commit-sources
git.folder = "scripts"                           # cd ../scripts
git.checkout(commit="{commit-scripts}")          # git checkout commit-scripts

I find this nicely explicit, easy to read and understand (anyone reading this code can understand the intention) and completely straightforward.

If the expectation is that Git() is a higher level abstraction of a repo, it isn't. It is a wrapper over the git command, and we think it is the right design and the plan is to keep it that way.

@fschoenm
Copy link
Author

@memsharded Sorry if that came off wrong, I didn't meant "imaginary" but "hypothetical".

I don't doubt those use cases exist (and it would be easily handled by having multiple Git objects instead of reusing it), it just isn't the typical straight-forward use case of having one Git repo and checking it out at a certain revision, which I feel the interface should be "optimized" for.

I also can see how your example would be very straight forward, it just isn't very abstract. In the end, what's the point of having a helper class that doesn't abstract (almost) anything away? I could just call git directly in that case.

I just wish Conan would have more high-level helper objects for handling such common use cases but in my recent experiments with Conan 2.x I have the feeling, it is developing in the opposite direction. Things like the scm attribute or the AutoPackager that were IMO very helpful, higher-level abstractions for common use cases/patterns are getting deprecated. Instead, everything has to be as explicit as possible and it just makes it feel like writing shell scripts sometimes.

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

Successfully merging a pull request may close this issue.

2 participants