◐ Shell
reader mode source ↗
Skip to content

bpo-28334: fix netrc not working when $HOME is not set#123

Closed
dmerejkowsky wants to merge 1 commit into
python:masterfrom
dmerejkowsky:netrc-home
Closed

bpo-28334: fix netrc not working when $HOME is not set#123
dmerejkowsky wants to merge 1 commit into
python:masterfrom
dmerejkowsky:netrc-home

Conversation

@dmerejkowsky

@dmerejkowsky dmerejkowsky commented Feb 15, 2017

Copy link
Copy Markdown

@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

  1. If you don't have an account on b.p.o, please create one
  2. Make sure your GitHub username is listed in "Your Details" at b.p.o
  3. If you have not already done so, please sign the PSF contributor agreement
  4. If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@dmerejkowsky

Copy link
Copy Markdown
Author

As discussed during patch review, no tests were added, but doc was updated.

@dmerejkowsky

Copy link
Copy Markdown
Author

@the-knights-who-say-ni :
Added my github username on bugs.python.org as requested.

@berkerpeksag berkerpeksag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Thanks for the PR, but like I said in review we can't accept this without a test case.

It should be easy to test at least the FileNotFoundError case by changing $HOME env variable.

@dmerejkowsky

Copy link
Copy Markdown
Author

Rebased, added a test, and fix doc style as requested.

@zhangyangyu zhangyangyu self-requested a review February 22, 2017 09:55
@zhangyangyu

Copy link
Copy Markdown
Member

LGTM. But I'd wait for others' reviews for some time. And I want to treat this as an enhancement though it looks like a bug on Windows.

@berkerpeksag berkerpeksag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Thanks! Looks good to me, just left some minor comments. Also, since this a behavior change we need to add a note to Misc/NEWS.

@dmerejkowsky

Copy link
Copy Markdown
Author

@berkerpeksag rebased and conformed with your latest comments

@dmerejkowsky dmerejkowsky force-pushed the netrc-home branch 2 times, most recently from df1f1bc to 09abcaf Compare April 10, 2017 11:28
@dmerejkowsky

dmerejkowsky commented Apr 10, 2017

Copy link
Copy Markdown
Author

@zhangyangyu Thanks for the suggestions! It's worth making sure the documentation is as best as possible :)
Rebased and changed as requested.

@dmerejkowsky

Copy link
Copy Markdown
Author

Wrote a test monkeypatching os.path.expanduser. Maybe we can keep it after all.
You tell me ;)

26 hidden items Load more…
@dmerejkowsky

Copy link
Copy Markdown
Author

@louisom

louisom commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

@dmerejkowsky For reminding, you don't need to squash your commit every change makes, all commit will be squash when the PR is merged. See devguide last paragraph.

@berkerpeksag

Copy link
Copy Markdown
Member

@dmerejkowsky and just one more comment: Please add your name to Misc/ACKS.

yan12125 pushed a commit to ytdl-org/youtube-dl that referenced this pull request May 29, 2017
khavishbhundoo added a commit to khavishbhundoo/youtube-dl that referenced this pull request Jun 14, 2017
* [cbsinteractive] fix extractor

* [cbsinteractive] update test cases

* [cbsinteractive] extract formats with `CBSIE`

* [extractor/common] Fix rtmp and rtsp formats' URLs in _extract_wowza_formats

* [vier] Extract more info

Extract the `episode_number` and `upload_date`. Also extract the real
`description`.

* [vier] Relax regexes and extract more metadata (closes #12539)

* [jsinterp] Add support for quoted names and indexers (closes #13123, closes #13130)

* [ChangeLog] Actualize

* release 2017.05.18

* [ChangeLog] Fix typo

* [jsinterp] Fix typo and cleanup regexes (closes #13134)

* [ChangeLog] Actualize

* release 2017.05.18.1

* [mitele] Update app key regex

* [hitbox] Add support for smashcast.tv (closes #13154)

* [njpwworld] Fix extraction (closes #13162)

* [toypics] Fix extraction

* [toypics] Improve and modernize

* [adobepass] Add support for Brighthouse MSO

* [toggle] Relax _VALID_URL (closes #13172)

* [youtube] Fix DASH manifest signature decryption (closes #8944)

* [youtube] Modernize

* [streamcz] Add support for subtitles

* [downloader/external] Pass -loglevel to ffmpeg downloader (closes #13183)

* Credit @zurfyx for atresplayer improvements (#12548)

* Credit @mphe for streamango (#12643)

* Credit @fredbourni for noovo (#12792)

* [ChangeLog] Actualize

* release 2017.05.23

* Credit @timendum for rai (#11790) and mediaset (#12964)

* Credit @gritstub for vevo fix (#12879)

* [cbsnews] fix extraction for 60 Minutes videos

* [vimeo] Fix formats' sorting (closes #13189)

* [postprocessor/ffmpeg] Fix metadata filename handling on Python 2

Fixes #13182

* [udemy] Fix extraction for outputs' format entries without URL (closes #13192)

* [youku] Fix extraction (closes #13191)

* [utils] Recognize more patterns in strip_jsonp()

Used in Youku Show pages

* [youku:show] Fix extraction

* [tudou] Merge into youku extractor (fixes #12214)

Also, there are no tudou playlists anymore. All playlist URLs points to youku
playlists.

* [bbc] Add support for authentication

* Revert "[youtube] Don't use the DASH manifest from 'get_video_info' if 'use_cipher_signature' is True (#5118)"

This reverts commit 87dc451.

* [ChangeLog] Update after the fix for #11381

* [ChangeLog] Actualize

* release 2017.05.26

* [cbsnews] Fix extraction (closes #13205)

* [youku] Extract more metadata (closes #10433)

* [adn] fix formats extraction

* [utils] Drop an compatibility wrapper for Python < 2.6

addinfourl.getcode is added since Python 2.6a1. As youtube-dl now
requires 2.6+, this is no longer necessary.

See python/cpython@9b0d46d

* [cbsinteractive] Relax _VALID_URL (closes #13213)

* [beam:vod] Add extractor

* [beam] Improve and add support for mixer.com (closes #13032)

* [dvtv] Parse adaptive formats as well

The old code hit an error when it attempted to parse the string
"adaptive" for video height. Actually parsing the returned playlists is
a good idea because it adds more output formats, including some
audio-only-ones.

* [dvtv] Improve and fix playlists support (closes #13063)

* [medialaan] Fix videos with missing videoUrl

A rough trick to get around the two different json styles medialaan seems to be using.
Fix for these example videos:
https://vtmkzoom.be/video?aid=45724
https://vtmkzoom.be/video?aid=45425

* [medialaan] PEP 8 (closes #12774)

* [gaskrank] Fix extraction

* [gaskrank] Improve (closes #12493)

* [abcnews] Add support for embed URLs

* [abcnews] Improve and remove duplicate test (closes #12851)

* [xhamster] Extract categories (closes #11728)

* [xhamster] Fix author and like/dislike count extraction

* [xhamster] Simplify (closes #13216)

* [youtube] Parse player_url if format URLs are encrypted or DASH MPDs are requested

Fixes #13211

* [ChangeLog] Actualize

* release 2017.05.29

* [README.md] Add an example for how to use .netrc on Windows

That's a Python bug: http://bugs.python.org/issue28334
Most likely it will be fixed in Python 3.7: python/cpython#123

* [README.md] Mention http_dash_segments protocol

* [packtpub] Fix authentication(closes #13240)

* [drbonanza] Fix extraction (closes #13231)

* [francetv] Relax _VALID_URL

* [1tv] Lower preference for http formats (closes #13246)

* [youtube] Improve chapters extraction (closes #13247)

* [safari] Fix typo (closes #13252)

* [YoutubeDL] Don't emit ANSI escape codes on Windows

* [godtv] Remove extractor (closes #13175)

* [pornhub:playlist] Fix extraction (closes #13281)

* [pornhub:uservideos] Add missing raise

* [bandcamp:weekly] Add extractor

* [bandcamp:weekly] Improve and extract more metadata (closes #12758)

* Credit @adamvoss for bandcamp:weekly (#12758)

* Credit @mikf for beam:vod (#13032)

* Credit @jktjkt for dvtv formats (#13063)

* [ChangeLog] Actualize

* release 2017.06.05

* [tvplayer] Fix extraction (closes #13291)

* [rtlnl] Improve _VALID_URL (closes #13295)

* [streamango] Make title optional

* [streamango] Skip download for test (closes #13292)

* [README.md] Clarify output template references (closes #13316)

* [README.md] Improve man page formatting

* [YoutubeDL] Sanitize more fields (#13313)

* [liveleak] Ensure height is int (closes #13313)

* [safari] Improve authentication detection (closes #13319)

* [sohu] Fix numeric fields

* [flickr] Ensure format id is string

* [foxgay] Ensure height is int

* [gfycat] Ensure filesize is int

* [golem] Ensure format id is string

* [jove] Ensure comment count is int

* [sexu] Ensure height is int

* [turbo] Ensure format id is string

* [extractor/common] Return unicode string from _match_id

* [extractor/generic] Ensure format id is unicode string

* [msn] Fix formats extraction

* [newgrounds] Improve formats and uploader extraction (closes #13346)

* [newgrounds:playlist] Add extractor (closes #10611)

* [utils] Improve unified_timestamp

* [newgrounds] Extract more metadata (closes #13232)

* [rutv] Add support for testplayer.vgtrk.com (closes #13347)

* [xfileshare] Modernize and pass referrer

* [xfileshare] Add support for rapidvideo (closes #13348)

* [compat] Introduce compat_HTMLParseError

* [utils] Handle HTMLParseError in extract_attributes (closes #13349)

* [xfileshare] PEP 8

* [ChangeLog] Actualize

* release 2017.06.12

* [compat] Add compat_HTMLParseError to __all__

* [corus] Add support for history.ca (closes #13359)

* [corus] Add support for showcase.ca
akruis pushed a commit to akruis/cpython that referenced this pull request Sep 9, 2017
akruis pushed a commit to akruis/cpython that referenced this pull request Sep 9, 2017
During interpreter shutdown Stackless kills tasklets on daemon threads, if they
have a non-trivial C-stack. This caused an assertion violation.

https://bitbucket.org/stackless-dev/stackless/issues/123
(grafted from 9fd4f6a9d266250bef5baa692b89516e73b00e62)
akruis pushed a commit to akruis/cpython that referenced this pull request Sep 9, 2017
@berkerpeksag

Copy link
Copy Markdown
Member

I've fixed merge conflicts, created NEWS entry with blurb and made some cosmetic changes. Is there anything left to do here?

Also add more tests to check that proper exceptions are raised
@berkerpeksag

Copy link
Copy Markdown
Member

For some reason, Travis CI didn't pick up latest changes:

/home/travis/build/python/cpython/Doc/library/pyexpat.rst:872:Footnote [#] is not referenced.

Serhiy fixed it a while ago and I don't get any warnings in my development environment.

@berkerpeksag

Copy link
Copy Markdown
Member

Ok, this looks like a Travis bug to me. I've opened PR #4537 and documentation build passed there.

@dmerejkowsky could you please create a new PR? If I merge PR #4537, GitHub overwrite author information and set me as author.

@dmerejkowsky

Copy link
Copy Markdown
Author

@dmerejkowsky could you please create a new PR? If I merge PR #4537, GitHub overwrite author information and set me as author.

It's not a problem for me :) It was very nice of you to finish up my pull request, so by all means just merge your own and let GitHub set you as author of the patch, you deserve it.

What matters to me is to be listed in the ACKS file 😛

@berkerpeksag

Copy link
Copy Markdown
Member

I've just merged PR #4537. Thank you for your work again, @dmerejkowsky. It took more than a year but we eventually fixed the bug :)

johnslavik pushed a commit to johnslavik/cpython that referenced this pull request Apr 8, 2026
PyREPL refactor: add more docstrings/comments
johnslavik pushed a commit to johnslavik/cpython that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants