GH-83417: Allow venv to add a .gitignore file to environments via a new scm_ignore_file parameter by brettcannon · Pull Request #108125 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation50 Commits12 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Member
•
edited by github-actionsbot
Loading
Off by default via code but on by default via the CLI, the .gitignore file contains * which causes the entire directory to be ignored.
that may beclosed by this pull request
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.
ambv approved these changes Aug 24, 2023
Looking at the 👍 on the comment from @ambv , people so far seem to prefer keeping the gitignore argument. Anyone prefer the approach suggested by @vsajip ?
I updated the PR using an scm_ignore_file parameter that takes as a string the name of the SCM to create an ignore file for. It dispatches to create_{scm_ignore_file}_ignore_file() for the file creation. I also made the CLI option be --without-scm-ignore-file. That should give @gaborbernat the flexibility he was after for virtualenv and anyone who customizing virtual environment creation via subclassing EnvBuilder.
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 EnvBuilder class. If there's a desire to add a CLI option in the future we can add that in. But honestly, while I was writing the tests for a --with-scm-ignore-file, I realized the effort was not worth it for the CLI right now, especially when even pyvenv.cfg wouldn't record the option as it's the default semantics and defaults are left out. So YAGNI, avoiding pre-mature API optimization, and better things to do with my time on a long weekend made me drop --with-scm-ignore-file.
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.
GH-83417: Allow GH-83417: Allow venv add a .gitignore file to environmentsvenv to add a .gitignore file to environments via a new scm_ignore_file parameter
I made the parameter take a single SCM name as no one realistically stores their source code in multiple version control systems
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.
If @ambv wants to drop his approval due to this change I would understand.
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.
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?
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.
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.
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 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 __init__() and create() however you want.
If it bothers people that much we could add a create_ignore_file() method which handles the dispatch to the appropriate create_*_ignore_file() method(s) and that lets you do whatever you want.
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.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot s390x RHEL7 LTO + PGO 3.x has failed when building commit e218e50.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/244/builds/5454) and take a look at the build logs.
- Check if the failure is related to this commit (e218e50) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/244/builds/5454
Failed tests:
- test.test_asyncio.test_subprocess
Failed subtests:
- test_subprocess_consistent_callbacks - test.test_asyncio.test_subprocess.SubprocessThreadedWatcherTests.test_subprocess_consistent_callbacks
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last): File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/test/test_asyncio/test_subprocess.py", line 788, in test_subprocess_consistent_callbacks self.loop.run_until_complete(main()) File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/asyncio/base_events.py", line 664, in run_until_complete return future.result() ^^^^^^^^^^^^^^^ File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/test/test_asyncio/test_subprocess.py", line 780, in main self.assertEqual(events, [ AssertionError: Lists differ: [('pi[29 chars]t'), 'pipe_connection_lost', ('pipe_data_recei[57 chars]ted'] != [('pi[29 chars]t'), ('pipe_data_received', 2, b'stderr'), 'pi[57 chars]ted']
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot PPC64LE RHEL7 3.x has failed when building commit e218e50.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/446/builds/3899) and take a look at the build logs.
- Check if the failure is related to this commit (e218e50) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/446/builds/3899
Failed tests:
- test.test_multiprocessing_fork.test_misc
Failed subtests:
- test_nested_startmethod - test.test_multiprocessing_fork.test_misc.TestStartMethod.test_nested_startmethod
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last): File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le/build/Lib/test/_test_multiprocessing.py", line 5475, in test_nested_startmethod self.assertEqual(results, [2, 1]) AssertionError: Lists differ: [1, 2] != [2, 1]
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️
Hi! The buildbot PPC64LE RHEL7 LTO + PGO 3.x has failed when building commit e218e50.
What do you need to do:
- Don't panic.
- Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
- Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/43/builds/4239) and take a look at the build logs.
- Check if the failure is related to this commit (e218e50) or if it is a false positive.
- If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.
You can take a look at the buildbot page here:
https://buildbot.python.org/all/#builders/43/builds/4239
Failed tests:
- test.test_asyncio.test_events
Failed subtests:
- test_create_connection_local_addr_nomatch_family - test.test_asyncio.test_events.PollEventLoopTests.test_create_connection_local_addr_nomatch_family
Summary of the results of the build (if available):
==
Click to see traceback logs
Traceback (most recent call last): File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-ppc64le.lto-pgo/build/Lib/test/test_asyncio/test_events.py", line 715, in test_create_connection_local_addr_nomatch_family with self.assertRaises(OSError): AssertionError: OSError not raised
This was referenced
Sep 18, 2023
csm10495 pushed a commit to csm10495/cpython that referenced this pull request
…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
hroncok added a commit to hroncok/virtualenv that referenced this pull request
hroncok added a commit to hroncok/virtualenv that referenced this pull request