Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged

Conversation

brettcannon
Copy link
Member

@brettcannonbrettcannon commentedAug 18, 2023
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.


📚 Documentation preview 📚:https://cpython-previews--108125.org.readthedocs.build/

ofek, edgarrmondragon, adamchainz, Glyphack, edmorley, and nikaro reacted with hooray emojiPierre-Sassoulas reacted with heart emoji
Off by default via code but on by default via the CLI, the `.gitignore` file contains `*` which causes the entire directory to be ignored.
@brettcannonbrettcannon linked an issueAug 18, 2023 that may beclosed by this pull request
brettcannonand others added2 commitsAugust 21, 2023 15:11
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@ambv
Copy link
Contributor

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.

hugovk, ofek, AA-Turner, brettcannon, and erlend-aasland reacted with thumbs up emojigaborbernat reacted with thumbs down emoji

@brettcannon
Copy link
MemberAuthor

Looking at the 👍 on the comment from@ambv , people so far seem to prefer keeping thegitignore argument. Anyone prefer the approach suggested by@vsajip ?

@brettcannon
Copy link
MemberAuthor

I updated the PR using anscm_ignore_file parameter that takes as a string the name of the SCM to create an ignore file for. It dispatches tocreate_{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 subclassingEnvBuilder.

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 customEnvBuilder 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 evenpyvenv.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.

ofek reacted with heart emoji

@brettcannonbrettcannon changed the titleGH-83417: Allowvenv add a.gitignore file to environmentsGH-83417: Allowvenv to add a.gitignore file to environments via a newscm_ignore_file parameterSep 4, 2023
@gaborbernat
Copy link
Contributor

gaborbernat commentedSep 4, 2023
edited
Loading

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.

@hugovk
Copy link
Member

hugovk commentedSep 4, 2023
edited by AA-Turner
Loading

I made the parameter take a single SCM name as no one realistically stores their source code in multiple version control systems.

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.gitignore.

(https://github.com/python/devinabox andhttps://github.com/pypa/distlib contain both a.hgignore and.gitgnore.https://git.launchpad.net/beautifulsoup/tree/ has both.bzrignore and.gitignore. I'm unsure if these are relics or still both in use.)

But I don't know if it's worth the extra complexity for these rare cases.

AA-Turner reacted with thumbs up emoji

@ambv
Copy link
Contributor

ambv commentedSep 4, 2023

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.

@ofek
Copy link
Contributor

ofek commentedSep 4, 2023

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.

edgarrmondragon reacted with thumbs up emoji

@brettcannon
Copy link
MemberAuthor

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?

@brettcannon
Copy link
MemberAuthor

Also, if we are talking about multiple SCMs, then that means you're already subclassing which means you can override__init__() andcreate() however you want.

If it bothers people that much we could add acreate_ignore_file() method which handles the dispatch to the appropriatecreate_*_ignore_file() method(s) and that lets you do whatever you want.

@gaborbernat
Copy link
Contributor

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.

@brettcannonbrettcannonenabled auto-merge (squash)September 15, 2023 22:14
@brettcannonbrettcannonenabled auto-merge (squash)September 15, 2023 22:14
@brettcannonbrettcannon merged commite218e50 intopython:mainSep 15, 2023
@brettcannonbrettcannon deleted the issue-83417-venv-gitignore branchSeptember 15, 2023 22:38
@brettcannon
Copy link
MemberAuthor

Thanks everyone!

ofek reacted with heart emoji

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbots390x RHEL7 LTO + PGO 3.x has failed when building commite218e50.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (e218e50) or if it is a false positive.
  5. 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", line788, intest_subprocess_consistent_callbacksself.loop.run_until_complete(main())  File"/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/asyncio/base_events.py", line664, inrun_until_completereturn future.result()^^^^^^^^^^^^^^^  File"/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/test/test_asyncio/test_subprocess.py", line780, inmainself.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']

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotPPC64LE RHEL7 3.x has failed when building commite218e50.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (e218e50) or if it is a false positive.
  5. 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", line5475, intest_nested_startmethodself.assertEqual(results, [2,1])AssertionError:Lists differ: [1, 2] != [2, 1]

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotPPC64LE RHEL7 LTO + PGO 3.x has failed when building commite218e50.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. 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.
  4. Check if the failure is related to this commit (e218e50) or if it is a false positive.
  5. 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", line715, intest_create_connection_local_addr_nomatch_familywithself.assertRaises(OSError):AssertionError:OSError not raised

csm10495 pushed a commit to csm10495/cpython that referenced this pull requestSep 28, 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>
hroncok added a commit to hroncok/virtualenv that referenced this pull requestNov 29, 2023
…venvSince Python 3.13, the .gitignore file is created by venv:python/cpython#108125Fixespypa#2670
hroncok added a commit to hroncok/virtualenv that referenced this pull requestNov 29, 2023
…venvSince Python 3.13, the .gitignore file is created by venv:python/cpython#108125Fixespypa#2670
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@AA-TurnerAA-TurnerAA-Turner left review comments

@edgarrmondragonedgarrmondragonedgarrmondragon left review comments

@AlexWaygoodAlexWaygoodAlexWaygood left review comments

@ambvambvambv approved these changes

@vsajipvsajipvsajip approved these changes

@gaborbernatgaborbernatgaborbernat approved these changes

@hugovkhugovkhugovk approved these changes

Assignees

@brettcannonbrettcannon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[venv] Adding a .gitignore file to virtual environments
10 participants
@brettcannon@ambv@gaborbernat@hugovk@ofek@bedevere-bot@vsajip@AA-Turner@edgarrmondragon@AlexWaygood

[8]ページ先頭

©2009-2025 Movatter.jp