Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
GH-83417: Allowvenv
to add a.gitignore
file to environments via a newscm_ignore_file
parameter#108125
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Off by default via code but on by default via the CLI, the `.gitignore` file contains `*` which causes the entire directory to be ignored.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Realistically no other version control systems are popular enough to warrant this feature. I like the discoverability of the current wording of the new arguments. The barrier of entry for a future version control system to become popular enough to warrant addition here is pretty high. Therefore, I don't think we need to bikeshed this anymore. Let's just add it so we can test it in practice. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I updated the PR using an I made the parameter take a single SCM name as no one realistically stores their source code in multiple version control systems. And since the ignore file prevents you from checking in the virtual environment, there's no need to commit a multitude of ignore files when developing a package. I stuck with an opt-out flag since there's only git support right now and you can't make the CLI use a custom Hopefully this works for everyone. The API becomes flexible while the CLI stays simple which I think covers everyone's key concerns. If@ambv wants to drop his approval due to this change I would understand. |
venv
add a.gitignore
file to environmentsvenv
to add a.gitignore
file to environments via a newscm_ignore_file
parametergaborbernat commentedSep 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Well this might be true, but imagine the case when I work at the company that for all internal projects uses SCM x. That being said, company still operates in a world where most upstream projects will use git. Shouldn't in this case a Hypothetically internal plugin by default disable version control on both SCM types? Or do you imagine that the caller will manually every time explicitly set the type of SCM the project in the current folder uses? The alternative would be too allow having multiple SCM targets in parallel. The additional SCM files for a different type of SCM would be automatically ignored by the matching SCMs configuration file, so there's no real downside in generating multiple such configurations, one per each SCM. Now granted even with the current design one could set this SCM to x,git as a string marker to generate both SCM configuration files but feels a bit odd to me. |
hugovk commentedSep 4, 2023 • edited by AA-Turner
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by AA-Turner
Uh oh!
There was an error while loading.Please reload this page.
It's not common, but there are repos that are stored in one SCM and mirrored to another, sometimes mirroring to GitHub to use the CI. It's hard to find one on request, but for examplehttps://sourceforge.net/p/docutils/code/HEAD/tree/ is SVN andhttps://repo.or.cz/docutils.git is Git, although this only contains a (https://github.com/python/devinabox andhttps://github.com/pypa/distlib contain both a But I don't know if it's worth the extra complexity for these rare cases. |
I don't mind. I don't have any fundamental reason against supporting many SCMs, I just think it's unnecessary to spend so much brain power on it. |
FWIW Hatchling supports multiple version control systems for default exclusion patterns:https://github.com/pypa/hatch/blob/hatchling-v1.18.0/backend/src/hatchling/builders/config.py#L798-L838 It's annoying to have in the code base but the risk of not supporting hypothetical users who would then open feature requests anyway is worse than the maintenance cost. |
But the file isn't getting committed to the repository just as the virtual environment isn't. So you only have to care about the version control system where you're creating the virtual environment which won't be multiple SCMs but just the one being used locally.
The caller can determine what it wants to specify however it chooses. Also note we are talking about companies that are big enough to be running two SCMs; they have the staff to support this. 😉 The question is whether we should optimize the API for all possibilities because we only expect people with non-standard SCMs to set anything, or do we optimize for the the 99% of the world who only operate under a single SCM? |
Also, if we are talking about multiple SCMs, then that means you're already subclassing which means you can override If it bothers people that much we could add a |
I don't think supporting 1+ SCMs is any more complicated than exactly 1 in this case, but the current ApI is a bit less user-friendly but I digress, I made my point. |
Uh oh!
There was an error while loading.Please reload this page.
Thanks everyone! |
bedevere-bot commentedSep 15, 2023
|
bedevere-bot commentedSep 16, 2023
|
bedevere-bot commentedSep 16, 2023
|
…ts via a new `scm_ignore_file` parameter (pythonGH-108125)This feature is off by default via code but on by default via the CLI. The `.gitignore` file contains `*` which causes the entire directory to be ignored.Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
…venvSince Python 3.13, the .gitignore file is created by venv:python/cpython#108125Fixespypa#2670
…venvSince Python 3.13, the .gitignore file is created by venv:python/cpython#108125Fixespypa#2670
Uh oh!
There was an error while loading.Please reload this page.
Off by default via code but on by default via the CLI, the
.gitignore
file contains*
which causes the entire directory to be ignored.📚 Documentation preview 📚:https://cpython-previews--108125.org.readthedocs.build/