bpo-38691 Added a switch to ignore PYTHONCASEOK when -E or -I flags passed by idomic · Pull Request #18314 · python/cpython

idomic · 2020-02-02T14:44:10Z

https://bugs.python.org/issue38691

idomic · 2020-02-02T14:48:40Z

@vstinner Added a PR 👍

idomic · 2020-02-02T15:54:24Z

Not sure why the test_importlib test fails, I think it expects the value no matter what?

corona10

Please run make regen-all before commit.
You can see some diffs which are generated from this command.
And please commit it after do this.

Thanks for the contribution

corona10 · 2020-02-03T09:48:04Z

Lib/importlib/_bootstrap_external.py

return not sys.flags.ignore_environment and key in _os.environ

Accepted both comments thank you!

Misc/NEWS.d/next/Library/2020-02-02-14-48-01.bpo-38691.ewUjZn.rst

vstinner · 2020-02-05T17:29:33Z

corona10 requested a review from vstinner 2 days ago

I will not review this change: most CIs are red. Please fix it before I can look at this PR.

idomic · 2020-02-08T21:26:58Z

@idomic

Please run make regen-all before commit.
You can see some diffs which are generated from this command.
And please commit it after do this.

Thanks for the contribution

@corona10
So I ran those 3, now I have a conflict on the importlib_external.h file:

git clean -fdx
./configure --with-pydebug
make regen-all

corona10 · 2020-02-09T03:56:06Z

@idomic
Please rebase before make regen-all

idomic · 2020-02-09T16:44:26Z

@corona10 I think I might be doing something wrong.

I was cleaning the folder using: git clean -fdx
Then rebasing master into current, running ./configure --with-pydebug and make regen-all.
Then I push to remote branch and letting it merge with remote.
I have also tried to merge just this conflicted importlib file from master and take the master's version but it failed the CI.

idomic · 2020-02-11T03:58:04Z

Adding @taleinat to accelerate PR

vstinner · 2020-02-11T11:56:22Z

This PR looks overcomplicated. It has 24 commits but only changes 1 line of code... Maybe create a new PR on top of the up to date master branch, and ensure that you run "make regen-all" to update Python/importlib_external.h.

taleinat · 2020-02-11T12:05:01Z

Hi @idomic!

I think I might be doing something wrong.

Yep, it seems that you've become a bit tangled up with the git merges and rebases. The final step that caused the current mess was when, after rebasing, you allowed merging with the remote branch. Instead, you should have force-pushed the rebased branch, using git push -f (use with care!).

I suggest you (hard-) reset your branch to the commit before merging the remote branch, and then force-push it.

(Generally, for branches with an open PR, I always recommend merging rather than rebasing. This is for several reasons, one of them being avoiding issues like this. Even when you're asked "please rebase", merging will usually achieve the same end result in terms of the files, but make following and managing the PR simpler.)

idomic · 2020-02-11T13:15:02Z

Thanks for the quick help @taleinat

So Once hard resetting and cherry picking the CI starts running, windows x64 fails on:
"D:\a\cpython\cpython\PCbuild_freeze_importlib.vcxproj(146,5): error : If you are not developing on Windows but you see this error on a continuous integration build, you need to run 'make regen-all' and commit any changes."

Once I run configure with --pydebug and make regen-all it creates a conflict in the file importlib_external.h even though I'm forcing my local branch push.

vstinner · 2020-02-11T13:19:39Z

Misc/NEWS.d/next/Library/2020-02-11-13-01-38.bpo-38691.oND8Sk.rst

Please avoid documenting private modules/functions in public Changelog entries. I propose:

The :mod:`importlib` module now ignore the :envvar:`PYTHONCASEOK` environment variable when :option:`-E` or :option:`-I` command line option is used.

Please document also this incompatible change at:

https://docs.python.org/dev/library/functions.html#__import__

https://docs.python.org/dev/whatsnew/3.9.html#changes-in-the-python-api

bedevere-bot · 2020-02-11T13:19:45Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

idomic · 2020-02-12T01:23:58Z

Thanks for the comments @vstinner
I have made the requested changes; please review again

bedevere-bot · 2020-02-12T01:24:02Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

vstinner · 2020-02-12T08:52:57Z

Doc/library/functions.rst

I suggest to add "now", suggestion which also added :envvar: markup:

When the command line options :option:`-E` or :option:`-I` is used, the environment variable :envvar:`PYTHONCASEOK` is now ignored.

vstinner · 2020-02-12T08:53:17Z

Doc/whatsnew/3.9.rst

Suggested change

* The :mod:`importlib` module now ignore the :envvar:`PYTHONCASEOK`

* The :mod:`importlib` module now ignores the :envvar:`PYTHONCASEOK`

I have made the requested changes; please review again

Well, you didn't update this one. English is not my first language, so I'm not sure, but it should be "The module ignoreS" here, no? with an S.

Yep, you're right regarding the English grammar here, @vstinner.

Misc/NEWS.d/next/Library/2020-02-11-13-01-38.bpo-38691.oND8Sk.rst

Doc/whatsnew/3.9.rst

idomic · 2020-02-13T19:37:50Z

I have made the requested changes; please review again

bedevere-bot · 2020-02-13T19:37:52Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

taleinat · 2020-02-13T21:46:15Z

Doc/library/functions.rst

Suggested change

When the command line options :option:`-E` or :option:`-I` is used,

When the command line options :option:`-E` or :option:`-I` are used,

Will fix both comments now

I understood what the problem was, apparently I misconfigured my local repo so there was no up stream, now everything should pass fine.

taleinat · 2020-02-13T21:47:39Z

Also, note that when you're done, you'll have to merge and regenerate Python/importlib_external.h one last time.

Co-Authored-By: Victor Stinner <[email protected]>

idomic · 2020-02-13T23:05:02Z

I have made the requested changes; please review again

bedevere-bot · 2020-02-13T23:05:05Z

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

codecov · 2020-02-16T19:06:21Z

Codecov Report

Merging #18314 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #18314     +/-   ##
=========================================
  Coverage   82.11%   82.12%             
=========================================
  Files        1956     1955      -1     
  Lines      589364   584024   -5340     
  Branches    44457    44457             
=========================================
- Hits       483962   479608   -4354     
+ Misses      95750    94766    -984     
+ Partials     9652     9650      -2

Impacted Files	Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py	`30.00% <0.00%> (-65.00%)`	⬇️
Lib/distutils/command/bdist_rpm.py	`7.63% <0.00%> (-56.88%)`	⬇️
Lib/test/test_urllib2net.py	`76.92% <0.00%> (-13.85%)`	⬇️
Lib/test/test_smtpnet.py	`78.57% <0.00%> (-7.15%)`	⬇️
Lib/ftplib.py	`63.85% <0.00%> (-6.06%)`	⬇️
Lib/test/test_ftplib.py	`87.11% <0.00%> (-4.72%)`	⬇️
Tools/scripts/db2pickle.py	`17.82% <0.00%> (-3.97%)`	⬇️
Tools/scripts/pickle2db.py	`16.98% <0.00%> (-3.78%)`	⬇️
Lib/test/test_socket.py	`71.94% <0.00%> (-3.77%)`	⬇️
Lib/test/test_asyncio/test_base_events.py	`91.84% <0.00%> (-3.30%)`	⬇️
... and 323 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5cbab5...c2c2315. Read the comment docs.

bedevere-bot · 2020-02-17T09:05:15Z

@vstinner: Please replace # with GH- in the commit message next time. Thanks!

vstinner · 2020-02-17T09:06:21Z

@idomic: Thanks, I merged your PR.

pablogsal · 2020-02-18T02:06:31Z

@vstinner This PR is likely to be the cause of https://buildbot.python.org/all/#/builders/275/builds/245

vstinner · 2020-02-18T13:37:06Z

@vstinner This PR is likely to be the cause of https://buildbot.python.org/all/#/builders/275/builds/245

Right: https://bugs.python.org/issue38691#msg362202

… flags passed (#18314)" (GH-18553) This reverts commit d83b660.

the-knights-who-say-ni added the CLA signed label Feb 2, 2020

bedevere-bot added the awaiting review label Feb 2, 2020

corona10 requested changes Feb 3, 2020

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Feb 3, 2020

corona10 requested changes Feb 3, 2020

View reviewed changes

corona10 requested a review from vstinner February 3, 2020 14:17

idomic force-pushed the PYTHONCASEOK branch 2 times, most recently from 334d852 to 3e872b2 Compare February 11, 2020 13:13

vstinner requested changes Feb 11, 2020

View reviewed changes

bedevere-bot removed the awaiting core review label Feb 11, 2020

bedevere-bot added the awaiting changes label Feb 11, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels Feb 12, 2020

bedevere-bot requested a review from vstinner February 12, 2020 01:24

vstinner reviewed Feb 12, 2020

View reviewed changes

taleinat reviewed Feb 12, 2020

View reviewed changes

Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved

bedevere-bot requested a review from vstinner February 13, 2020 19:37

taleinat reviewed Feb 13, 2020

View reviewed changes

idomic and others added 4 commits February 13, 2020 17:56

Hard reset + cherry piciking the changes.

13c5b05

📜🤖 Added by blurb_it.

fdecdf0

Added @vstinner News

29d13c9

Update Misc/NEWS.d/next/Library/2020-02-11-13-01-38.bpo-38691.oND8Sk.rst

135259f

Co-Authored-By: Victor Stinner <[email protected]>

idomic added 4 commits February 14, 2020 20:10

Merge branch 'master' of https://github.com/python/cpython

794b948

Merge branch 'master' of https://github.com/python/cpython

bfa3cd1

Hard reset to master

cfea4b2

Hard reset to master + latest changes

c2c2315

idomic force-pushed the PYTHONCASEOK branch from 6a22f97 to c2c2315 Compare February 16, 2020 18:36

vstinner merged commit d83b660 into python:master Feb 17, 2020

bedevere-bot removed the awaiting change review label Feb 17, 2020

idomic deleted the PYTHONCASEOK branch February 17, 2020 12:16

vstinner added a commit that referenced this pull request Feb 19, 2020

Revert "bpo-38691 Added a switch to ignore PYTHONCASEOK when -E or -I…

4dee92b

… flags passed (#18314)" (GH-18553) This reverts commit d83b660.

vstinner mentioned this pull request Feb 29, 2020

bpo-38691: Ignore pythoncaseok tests #18627

Merged

	* The :mod:`importlib` module now ignore the :envvar:`PYTHONCASEOK`
	* The :mod:`importlib` module now ignores the :envvar:`PYTHONCASEOK`

	When the command line options :option:`-E` or :option:`-I` is used,
	When the command line options :option:`-E` or :option:`-I` are used,

Uh oh!

Comments

Conversation

idomic commented Feb 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

idomic commented Feb 2, 2020

Uh oh!

idomic commented Feb 2, 2020

Uh oh!

corona10 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner commented Feb 5, 2020

Uh oh!

idomic commented Feb 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corona10 commented Feb 9, 2020

Uh oh!

idomic commented Feb 9, 2020

Uh oh!

idomic commented Feb 11, 2020

Uh oh!

vstinner commented Feb 11, 2020

Uh oh!

taleinat commented Feb 11, 2020

Uh oh!

idomic commented Feb 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Feb 11, 2020

Uh oh!

idomic commented Feb 12, 2020

Uh oh!

bedevere-bot commented Feb 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

idomic commented Feb 13, 2020

Uh oh!

bedevere-bot commented Feb 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taleinat commented Feb 13, 2020

Uh oh!

idomic commented Feb 13, 2020

Uh oh!

bedevere-bot commented Feb 13, 2020

Uh oh!

codecov bot commented Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bedevere-bot commented Feb 17, 2020

Uh oh!

vstinner commented Feb 17, 2020

Uh oh!

pablogsal commented Feb 18, 2020

idomic commented Feb 2, 2020 •

edited

Loading

corona10 left a comment •

edited

Loading

idomic commented Feb 8, 2020 •

edited

Loading

idomic commented Feb 11, 2020 •

edited

Loading

codecov bot commented Feb 16, 2020 •

edited

Loading