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

Improve behavior of dulwich.porcelain.add #895

Open
dclong opened this issue Aug 7, 2021 · 4 comments
Open

Improve behavior of dulwich.porcelain.add #895

dclong opened this issue Aug 7, 2021 · 4 comments

Comments

@dclong
Copy link

dclong commented Aug 7, 2021

By default, dulwich.porcelain.add add files from the current working directory rather the root directory of the specified repository. This can be confirm by the highlighted code in the screenshot below. I personally think it is better to add files from the root directory of the repository by default.
image

@dclong
Copy link
Author

dclong commented Aug 7, 2021

What's more, if the current working directory of Python is not /tmp/test_dulwich, the following code doesn't add anything into stage.

dulwich.porcelain.add("/tmp/dulwich", "/tmp/dulwich")

This is rather inconvenient if users have to work across multiple local repositories.

@jelmer
Copy link
Owner

jelmer commented Aug 7, 2021

The idea is that "porcelain.add" works the same way that "git add" does - in the same way that other commands in the porcelain module do. This means behaviour relative to the current working directory. I'm open to suggestions for improvements that make this less confusing. If you want a lower level interface, Repo.stage is the right function to call.

@AlpBilgin
Copy link

AlpBilgin commented Oct 23, 2021

Hello @jelmer,

I came here to complain about the same behavior, then I figured it out after reading your message and thinking "oh this sounds like I missed something in the docs."

Assuming I make a wrapper with convenience functions:

from dulwich import porcelain
class GitClient:
    def __init__(self, path: str, remote: str,):
        self.remote = remote
        self.path = path
        self.repo = porcelain.clone(remote, path)


    def git_add_commit_push(self, commit_message):
        porcelain.add(self.repo, self.path)
        porcelain.commit(self.repo, commit_message)
        porcelain.push(self.repo, self.remote)

it isn't immediately obvious from the doc wording that add() will scan anything and everything under CWD, if I don't pass self.path which is where I initialized the repo.

I assume there is a good reason why this is the default behavior, but it is very counterintuitive to try and add files that are outside of the repo that the self.repo pointer is pointing to.

It might be reasonable to warn users in the docs just in case.

edit: after fiddling around a bit more I noticed that giving the folder path doesn't actually work. It seemed to work because I am using this for a very specific use case and it is possible to make this work for my use case due to a side effect from my badly designed tests. For the reader I suggest enumerating the dirty files in the target dir and giving them to .add() as an explicit list.

@jelmer
Copy link
Owner

jelmer commented Oct 25, 2021

dulwich.porcelain is meant to provide the same behaviour as the "git" command-line tool, which also takes into account the current working directory.

If you're after lower level access, use the dulwich.repo.Repo object, which has a stage() method.

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