Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-103186: In test_tools.freeze, fetch CONFIG_ARGS from original source directory #103213

Merged

Conversation

TabLand
Copy link
Contributor

@TabLand TabLand commented Apr 3, 2023

instead of fetching from intermediate instance. As "make clean" is called against the intermediate instance, the build directory is cleared and the config arguments lookup fails with a ModuleNotFoundError:

# /opt/python/test/cpython/python -c 'import sysconfig; print(sysconfig.get_config_var("CONFIG_ARGS"))'
CalledProcessError: Command '['/opt/python/test/cpython/python', '-c', 'import sysconfig; print(sysconfig.get_config_var("CONFIG_ARGS"))']' returned non-zero exit status 1.
--- STDOUT ---

--- STDERR ---
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/python/test/cpython/Lib/sysconfig.py", line 740, in get_config_var
    return get_config_vars().get(name)
           ^^^^^^^^^^^^^^^^^
  File "/opt/python/test/cpython/Lib/sysconfig.py", line 723, in get_config_vars
    _init_config_vars()
  File "/opt/python/test/cpython/Lib/sysconfig.py", line 670, in _init_config_vars
    _init_posix(_CONFIG_VARS)
  File "/opt/python/test/cpython/Lib/sysconfig.py", line 536, in _init_posix
    _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named '_sysconfigdata_d_linux_x86_64-linux-gnu'

More context here: #103186 (comment)

instead of fetching from intermediate instance. As "make clean" is called
against the intermediate instance, the build directory is cleared and the
config arguments lookup fails with a ModuleNotFoundError
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@TabLand
Copy link
Contributor Author

TabLand commented Apr 3, 2023

Hello @erlend-aasland: I saw you worked on a similar change recently, would appreciate any feedback.

Hello freeze experts: There is probably another way to capture the same info from the intermediate instance as long as we do so before "make clean" is called - but that solution felt messy. Please let me know whether I should have a go at improving the variable naming here as SRCDIR (original), srcdir (intermediate) and builddir (frozen) could cause confusion...

@arhadthedev arhadthedev added the build The build process and cross-build label Apr 3, 2023
@arhadthedev
Copy link
Member

@terryjreedy (as a participant of the parent issue)

@TabLand
Copy link
Contributor Author

TabLand commented Apr 3, 2023

The following new exception for missing make file in the intermediate python instance is passing on all platforms except Ubuntu as part of the Github CI pipeline builds:

     if os.path.exists(os.path.join(newroot, 'Makefile')):
         _run_quiet([MAKE, 'clean'], newroot)
+    else:
+        raise Exception("new frozen build Makefile missing!")

I'm happy to remove this, as technically the existence of the Makefile depends on ./configure being run successfully first?

It depends on ./configure
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@arhadthedev
Copy link
Member

Closing as a duplicate of gh-102152.

@erlend-aasland
Copy link
Contributor

Closing as a duplicate of gh-102152.

This is not a duplicate of gh-102152, which fetched CONFIG_ARGS from the copied source directory instead of from the build directory. This PR fetches CONFIG_ARGS from the original source directory, so it is slightly different, and possibly more correct; I don't have the bandwidth to look at this right now. I'll definitely be able to review it around the summit in a couple of weeks.

@TabLand
Copy link
Contributor Author

TabLand commented Jul 8, 2023

Hi @erlend-aasland / @terryjreedy,
Appreciate you're very busy...
Please let me know your thoughts on this PR?

@terryjreedy
Copy link
Member

This PR involves things beyond my knowledge. Erland, if you cannot finish this, can you suggest another reviewer?

@erlend-aasland erlend-aasland changed the title gh-103186 - Fetch CONFIG_ARGS from original python instance gh-103186: In test_tools.freeze, fetch CONFIG_ARGS from original source directory Jul 11, 2023
@erlend-aasland erlend-aasland enabled auto-merge (squash) July 11, 2023 21:55
@erlend-aasland
Copy link
Contributor

@kumaraditya303, should we backport this to 3.12?

@erlend-aasland erlend-aasland merged commit de82732 into python:main Jul 11, 2023
17 checks passed
@kumaraditya303
Copy link
Contributor

should we backport this to 3.12?

Yes

@kumaraditya303 kumaraditya303 added skip news needs backport to 3.12 bug and security fixes and removed skip news labels Jul 12, 2023
@miss-islington
Copy link
Contributor

Thanks @TabLand for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-106667 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Jul 12, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 12, 2023
…l source directory (pythonGH-103213)

Fetch CONFIG_ARGS from the original source directory, instead of from
the copied source tree. When "make clean" is executed in the copied
source tree, the build directory is cleared and the configure argument
lookup fails. However, the original source directory still contains this
information.
(cherry picked from commit de82732)

Co-authored-by: Ijtaba Hussain <[email protected]>
kumaraditya303 pushed a commit that referenced this pull request Jul 12, 2023
…al source directory (GH-103213) (#106667)

gh-103186: In test_tools.freeze, fetch CONFIG_ARGS from original source directory (GH-103213)

Fetch CONFIG_ARGS from the original source directory, instead of from
the copied source tree. When "make clean" is executed in the copied
source tree, the build directory is cleared and the configure argument
lookup fails. However, the original source directory still contains this
information.
(cherry picked from commit de82732)

Co-authored-by: Ijtaba Hussain <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants