Silently opens repo for ancestor directory

[eyevz]

Let /git be a git repo, suppose path /git/foo/bar exists (file or directory).

>>> git.Repo('/git/foo/bar')
<git.Repo "/git/.git">

I think this is very dangerous behaviour. For example, in a testing environment one might use

shutil.rmtree(some_repo.working_dir)

where, if some_repo was subject to the behaviour above, created from a path in a project's working directory, unhappiness might ensue.

[Byron]

Thanks for raising this issue ! I absolutely agree.

This behavior should be made optional, and be off by default in future versions, stating the change clearly in the change-notes.

Besides, I do hope you didn't accidentally delete some of your data !

[eyevz]

Nothing so serious - I only mangled a few refs!

Today I have to attend to something else, but tomorrow I will be back on a project where I'm using your library, so I will make the change as you suggest, with a note and some tests, and create a pull-request.

I'm thinking Repo.init should get a new keyword argument, maybe "aggressive=True", which retains the current behaviour. But if aggressive == False then only the parent of .git directory or .git itself are acceptable targets.

Your thoughts? I'm not 100% happy with the name "aggressive". Let me know if you think of a more natural name.

[eyevz]

Please note additional comments hidden behind the "..." button in my previous message. I'm not sure how I did that :)

[Byron]

I would prefer a KW argument name too, maybe one which is more descriptive and precise, like search_parent_directories=True for instance. Its a long name, but it says what it does pretty well.
But, please pick one to your liking in case you find a shorter and equally descriptive name.
Cheers

[sigmavirus24]

Wouldn't legacy be more descriptive, i.e., this is the old behavior and if that's what you want to expect, than you want the Repo object to operate in "legacy mode"?

[eyevz]

Okay, I will use search_parent_directories for now. The docs and change log will explain that default behaviour will change at some point. We can discuss further in the pull-request.

I see that master is quite different from 0.3 branch. Against which should I make the change? Both?

Finally, I am having trouble running the tests (in both branch of the mentioned branches). I will take that discussion to the mailing list.

[Byron]

I would recommend putting it into master, even though its rather far away from a release.
In 0.3, I wouldn't know if anyone would actually use it or require it. However, if you would like it there, as you would use it in 0.3, thats all I need to justify its presence in both branches.

Thank you.

[Byron]

The keyword was added as discussed.

Additionally, I recorded the process of fixing this one

• Part 1 on youtube
• Part 2 on youtube