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

Don't try to add ignored files to git. #196

Closed
wants to merge 1 commit into from

Conversation

kwmiebach
Copy link

@kwmiebach kwmiebach commented Aug 13, 2023

If I start aider with a file that is in .gitignore and should be ignored by git, aider tries nonetheless to git add the file. I think this is an obvious bug.

Old behaviour:

Let's say files aa and bb are in .gitignore:

$ cat .gitignore
aa
bb
.aider*

Now let's say the user starts aider and gives aa and bb as file parameters:

appuser@73ab230c4d0a:/src$ aider aa bb

The old behaviour is that aider will immediateley try to git add both files. And this results in a python exception:

Aider v0.12.1-dev
Model: gpt-4
Git repo: .git
Repo-map: universal-ctags using 1024 tokens
Added aa to the chat.
Added bb to the chat.
Adding /src/aa to git
Traceback (most recent call last):
  File "/opt/venv/bin/aider", line 33, in <module>
    sys.exit(load_entry_point('aider-chat', 'console_scripts', 'aider')())
  File "/src/aider/main.py", line 479, in main
    coder = Coder.create(
  File "/src/aider/coders/base_coder.py", line 84, in create
    return EditBlockCoder(main_model, io, **kwargs)
  File "/src/aider/coders/editblock_coder.py", line 14, in __init__
    super().__init__(*args, **kwargs)
  File "/src/aider/coders/base_coder.py", line 206, in __init__
    self.repo.add_new_files(fname for fname in fnames if not Path(fname).is_dir())
  File "/src/aider/repo.py", line 60, in add_new_files
    self.repo.git.add(fname)
  File "/opt/venv/lib/python3.10/site-packages/git/cmd.py", line 741, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/opt/venv/lib/python3.10/site-packages/git/cmd.py", line 1315, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/opt/venv/lib/python3.10/site-packages/git/cmd.py", line 1109, in execute
    raise GitCommandError(redacted_command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(1)
  cmdline: git add /src/aa
  stderr: 'The following paths are ignored by one of your .gitignore files:
aa
hint: Use -f if you really want to add them.
hint: Turn this message off by running
hint: "git config advice.addIgnoredFile false"'

In the other case, if a file is not git-ignored and it is also not part of the git index, aider will correctly add it to git:

Example for untracked file (not ignored):

$ touch cc
appuser@73ab230c4d0a:/src$ aider cc
Aider v0.12.1-dev
Model: gpt-4
Git repo: .git
Repo-map: universal-ctags using 1024 tokens
Added cc to the chat.
Adding /src/cc to git
Use /help to see in-chat commands, run with --help to see cmd line args
Git repo has uncommitted changes.
diff --git a/cc b/cc
new file mode 100644
index 0000000..e69de29
Commit before the chat proceeds [y/n/commit message]? y
Commit f143e86 Added new file cc.
────
cc>

In the proposed fix, before git adding a file, the code would use the repo.ignored() check and if the file is ignored it would not try to add the file to git:

$ aider aa bb
Aider v0.12.1-dev
Model: gpt-4
Git repo: .git
Repo-map: universal-ctags using 1024 tokens
Added aa to the chat.
Added bb to the chat.
Use /help to see in-chat commands, run with --help to see cmd line args
────────
aa bb>

But there is a small issue with this solution: /ls does not list aa and bb. I could not find out why. So while my proposed change fixeds one obvious thing, it introduces another problem.

Results in a git error and an exception
otherwise.
@kwmiebach
Copy link
Author

kwmiebach commented Aug 13, 2023

(was merged into the first comment)

@buger
Copy link

buger commented Aug 14, 2023

Generated by AI, can contain issues

User Experience

When an ignored file isn't added, can you print a message explaining why for the user? This improves perceived transparency.

Something like:

File .env is ignored - not adding to repository

Testing

Here is how I would adjust the test case to add to the tests/test_repo.py file

def test_add_new_files_ignored(self):

  # Configure git to show ignored file warnings
  self.repo.git.config("advice.addIgnoredFile", "false")

  # Add some entries to the .gitignore
  with open(self.repo.working_dir / ".gitignore", "w") as f:
    f.write("ignored.txt\n")

  # Try to add an ignored file
  self.repo.add_new_files(["ignored.txt"])

  # Verify the ignored file was not added
  tracked_files = self.repo.get_tracked_files()
  self.assertNotIn("ignored.txt", tracked_files)

The key points:

  • Use the existing self.repo instance to avoid recreating a repository

  • Disable warnings about adding ignored files to prevent errors

  • Add an entry to .gitignore for the test file

  • Call add_new_files on the ignored file path

  • Check the file was not added with get_tracked_files

This follows the same overall pattern as the other tests - use the repo instance, call the method being tested, and assert the expected behavior.

Let me know if you have any other questions!

@buger
Copy link

buger commented Aug 14, 2023

Something from the human 😅. I wonder if sometimes it expected to add ignored files, will be cool for /add command to have some argument like git add has e.g. "--force". But it probably can be next milestone, if there will be demand from users.

@kwmiebach
Copy link
Author

kwmiebach commented Aug 16, 2023

When an ignored file isn't added, can you print a message explaining why for the user? This improves perceived transparency.

@buger why should this happen? Did you think it through? It creates unneccessary noise.

Maybe in different use cases. If I tell aider to add the file to git, it would maybe help to get a message. But in my case I simply want to work on the file.

Maybe you want to explain to me how I could distinguish the 2 cases in the code.

@kwmiebach
Copy link
Author

Maybe I am understanding aider wrong.

I can work on all files as a human, and aider should also be able to work on all files and see them, especially if I tell it to use them.

But only files that are not ignored should be tracked by git.

Or otherwise, how would I give aider information from files that are git ignored?

@kwmiebach
Copy link
Author

I found more problems with the proposed change. Aider can no longer list and understand which files are in git.

I believe now aider was built around the idea that files it can edit and files in the git repo are basically the same.

I am closing this.

@kwmiebach kwmiebach closed this Aug 16, 2023
@buger
Copy link

buger commented Aug 16, 2023

@kwmiebach im not official contribution or owner of this repo, so take my words and thoughts with a bit of salt. Overall idea of this pr totally makes sense. But sometimes ignored files are not just some config files or similar, it can be code which you do not want be changed, e.g it’s in repo but can’t be easily changed.

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

Successfully merging this pull request may close these issues.

3 participants