GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-29573: Fixed unlinking already removed NamedTemporaryFile. by andrewnester · Pull Request #134 · python/cpython

Conversation

@andrewnester

@andrewnester andrewnester commented Feb 16, 2017 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fix for http://bugs.python.org/issue29573

https://bugs.python.org/issue29573

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 16, 2017
briancurtin
briancurtin requested changes Feb 16, 2017
View reviewed changes

@briancurtin briancurtin left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, just needs a small test change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_tempfile.py Outdated

@briancurtin briancurtin Feb 16, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add a self.addCleanup on that tmpdir—see the test above (test_bad_mode) for an example, just to make sure that if this test ever does fail that we don't have a temp dir that doesn't get cleaned up by us.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@andrewnester andrewnester Feb 16, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@briancurtin right, thanks! fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@andrewnester andrewnester May 12, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@briancurtin could you please review it again, it's fixed now

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@andrewnester
bpo-29573 - Fixed unlinking already removed NamedTemporaryFile.
d7edb94
Yhg1s
Yhg1s requested changes May 22, 2017
View reviewed changes
Comment thread Lib/test/test_tempfile.py
tmpdir = tempfile.mkdtemp()
self.addCleanup(support.rmtree, tmpdir)
with tempfile.NamedTemporaryFile(delete=True, dir=tmpdir) as fp:
os.system('rm {}'.format(fp.name))

@Yhg1s Yhg1s May 22, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this use os.system to call rm, rather than calling os.unlink? 'rm' is not exactly cross-platform. This test will also fail on Windows (and other systems where you can't delete files that are currently in use). It'll have to be skipped on those platforms.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Anselm Kruis
Stackless issue python#134: use Py_TYPE, Py_SIZE and Py_REFCNT
c7dcca5 
Replace the direct access to PyObject members with the preferred macros.
@brettcannon

brettcannon commented Feb 21, 2018

Copy link
Copy Markdown
Member

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon brettcannon added the awaiting changes label Feb 21, 2018
@serhiy-storchaka serhiy-storchaka changed the title bpo-29573 - Fixed unlinking already removed NamedTemporaryFile. bpo-29573: Fixed unlinking already removed NamedTemporaryFile. Mar 26, 2018
akruis pushed a commit to akruis/cpython that referenced this pull request Jun 19, 2018
Anselm Kruis
Stackless issue python#134: use Py_TYPE, Py_SIZE and Py_REFCNT
b842f61 
Replace the direct access to PyObject members with the preferred macros.
(cherry picked from commit c7dcca5)
@csabella
csabella requested a review from briancurtin May 20, 2019 00:56
@flavianh

flavianh commented Jul 21, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

@briancurtin @Yhg1s @brettcannon Is this PR worth being revived? The user's repo does not exist any more. I could take care of it by creating a new PR if you want
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon brettcannon closed this Jul 26, 2019
@brettcannon

brettcannon commented Jul 26, 2019

Copy link
Copy Markdown
Member

@flavianh up to you if you want to create a new PR to try and solve the problem.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jaraco pushed a commit that referenced this pull request Dec 2, 2022
@pyup-bot @Mariatta
Update cherry_picker from 1.2.0 to 1.2.1 (#134)
bcb7c62
@gvanrossum gvanrossum mentioned this pull request Aug 22, 2023
heap-use-after-free in _PyFunction_LookupByVersion #108253
Closed
This was referenced Feb 11, 2025
segmentfault when pip installing setuptools #129992
Closed
segmentfault when pip installing setuptools #129993
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yhg1s Yhg1s Yhg1s requested changes
@briancurtin briancurtin Awaiting requested review from briancurtin

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@andrewnester @brettcannon @flavianh @briancurtin @Yhg1s @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.