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

SCons: Bump min version to 3.1.2, test it on CI with one Linux job #92043

Merged
merged 1 commit into from
May 17, 2024

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented May 17, 2024

The min SCons version had to be bumped as SCons 3.0 before 3.0.3 seems broken (see below), and there's little gain from supporting 3.0.3-3.0.5.

3.1.2 is also the first version to avoid ambiguities between Python 2 and Python 3 usage, so we finally use it as the minimum baseline.

Also test against Python 3.6 which is also our minimum supported version.
This should help prevent regressions whenever we modernize the build scripts.

Also a bunch of cosmetic changes, got carried away :P

This should fail initially because of #91986, so I will rebase after #92041 is merged.
Confirmed, so #92041 is now included in this PR.

CC @Repiteo @AThousandShips

@akien-mga akien-mga added this to the 4.3 milestone May 17, 2024
@akien-mga akien-mga requested a review from a team as a code owner May 17, 2024 07:29
@akien-mga akien-mga force-pushed the ci-scons-3.0.0-python-3.6 branch 2 times, most recently from 2be3045 to 10d518d Compare May 17, 2024 07:49
shell: bash
run: |
python -c "import sys; print(sys.version)"
python -m pip install scons==4.7.0
python -m pip install wheel
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to install wheel before scons, otherwise it failed with this error:

3.6.15 (default, Sep  6 2021, 07:07:23) 
[GCC 9.3.0]
Collecting scons==3.0.0
  Downloading scons-3.0.0.tar.gz (630 kB)
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
Using legacy 'setup.py install' for scons, since package 'wheel' is not installed.
Installing collected packages: scons
    Running setup.py install for scons: started
    Running setup.py install for scons: finished with status 'error'
    ERROR: Command errored out with exit status 1:
     command: /opt/hostedtoolcache/Python/3.6.15/x64/bin/python -u -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-c84v8i3g/scons_e21b17358cca4fa0a6913eec3e36b727/setup.py'"'"'; __file__='"'"'/tmp/pip-install-c84v8i3g/scons_e21b17358cca4fa0a6913eec3e36b727/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-9y39lo61/install-record.txt --single-version-externally-managed --compile --install-headers /opt/hostedtoolcache/Python/3.6.15/x64/include/python3.6m/scons
         cwd: /tmp/pip-install-c84v8i3g/scons_e21b17358cca4fa0a6913eec3e36b727/
    Complete output (6 lines):
    usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
       or: setup.py --help [cmd1 cmd2 ...]
       or: setup.py --help-commands
       or: setup.py cmd --help
    
    error: option --single-version-externally-managed not recognized
    ----------------------------------------
ERROR: Command errored out with exit status 1: /opt/hostedtoolcache/Python/3.6.15/x64/bin/python -u -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-c84v8i3g/scons_e21b17358cca4fa0a6913eec3e36b727/setup.py'"'"'; __file__='"'"'/tmp/pip-install-c84v8i3g/scons_e21b17358cca4fa0a6913eec3e36b727/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-9y39lo61/install-record.txt --single-version-externally-managed --compile --install-headers /opt/hostedtoolcache/Python/3.6.15/x64/include/python3.6m/scons Check the logs for full command output.

@akien-mga
Copy link
Member Author

Confirmed that the build fails as expected without #92041: https://github.com/godotengine/godot/actions/runs/9124804393/job/25089728389?pr=92043

Building with flags: platform=linuxbsd target=editor tests=true verbose=yes warnings=extra werror=yes module_text_server_fb_enabled=yes dev_build=yes use_asan=yes use_ubsan=yes use_llvm=yes linker=lld
scons: Reading SConscript files ...
TypeError: 'SConsEnvironment' object is not a mapping:
  File "/home/runner/work/godot/godot/SConstruct", line 368:
    opts.Update(env, {**ARGUMENTS, **env})

I'll rebase to include that PR.

@akien-mga akien-mga force-pushed the ci-scons-3.0.0-python-3.6 branch from 10d518d to d294d8d Compare May 17, 2024 07:55
@AThousandShips
Copy link
Member

AThousandShips commented May 17, 2024

Looks great and very helpful, approving on principle but needs to pass compilation, something with the configuration fixes if the environment I suspect

Could be specific to 3.0.0, might be worth trying with 3.1.2 which has some changes, like python dependencies, unsure

@akien-mga
Copy link
Member Author

akien-mga commented May 17, 2024

Indeed, that's a weird error:

Checking for C header file mntent.h... TypeError : sequence item 0: expected str instance, bytes found

https://github.com/godotengine/godot/actions/runs/9124860447/job/25089907695?pr=92043

I can't reproduce it locally with a python:3.6 environment in podman, so not sure how to debug.
Assuming the error does happen on the SCons CheckHeader method, it's not something we can patch ourselves.

I'll throw a few tests on GitHub actions in my fork to see if newer Python or SCons solves it.

So in short, we could makek SCons 3.0.3 the min supported version to work around this bug.

Based on https://repology.org/project/scons/versions, that would exclude Debian 10 and derivatives which have SCons 3.0.1, but I think it's fine by now. Debian 10 will be EOL in a month: https://wiki.debian.org/LTS

We don't gain much from supporting 3.0.3-3.0.5 though, hardly any distro provides it.
Quite a few distros have 3.1.2, some have 3.1.1, none seems to have 3.1.0 but if it works, I think we can use it as the baseline.

@AThousandShips
Copy link
Member

I think it'd be worth raising the minimum version, unless that excludes a large number of environments, like automated tools somewhere, but otherwise I'd say it isn't worth the hassle to plug holes and provide workarounds

I think large distros like LTS Ubuntu etc. are a good reference especially when automatic configuration etc. is involved like CI and containers

@akien-mga akien-mga force-pushed the ci-scons-3.0.0-python-3.6 branch from d294d8d to 07458d7 Compare May 17, 2024 08:57
@akien-mga akien-mga changed the title CI: Use SCons 3.0.0 and Python 3.6 for one Linux build SCons: Bump min version to 3.1.0, test it on CI with one Linux job May 17, 2024
@akien-mga
Copy link
Member Author

I bumped the minimum SCons version to 3.1.0 as described above.

@AThousandShips
Copy link
Member

AThousandShips commented May 17, 2024

Could be worth considering bumping to 3.1.2 given the note in the build instructions (they will also need updating, with 3.1.0 at least):

If your distribution uses Python 2 by default, or you are using a version of SCons prior to 3.1.2, you will need to change the version of Python that SCons uses by changing the shebang (the first line) of the SCons script file to #! /usr/bin/python3. Use the command which scons to find the location of the SCons script file.

The min SCons version had to be bumped as SCons 3.0 before 3.0.3 seems
broken (see godotengine#92043), and there's little gain from supporting 3.0.3-3.0.5.

3.1.2 is also the first version to avoid ambiguities between Python 2
and Python 3 usage, so we finally use it as the minimum baseline.

Also test against Python 3.6 which is also our minimum supported version.
This should help prevent regressions whenever we modernize the build scripts.
@akien-mga akien-mga force-pushed the ci-scons-3.0.0-python-3.6 branch from 07458d7 to a63c37d Compare May 17, 2024 09:19
@akien-mga akien-mga changed the title SCons: Bump min version to 3.1.0, test it on CI with one Linux job SCons: Bump min version to 3.1.2, test it on CI with one Linux job May 17, 2024
@akien-mga
Copy link
Member Author

Bumped to 3.1.2 in the end.
Next stop, 4.0.0, sometime next year maybe?

@AThousandShips
Copy link
Member

That sounds reasonable I think, it'll be 5 years old by then and I think we can treat that as a reasonable backwards range

@akien-mga akien-mga merged commit bd2300d into godotengine:master May 17, 2024
16 checks passed
@akien-mga akien-mga deleted the ci-scons-3.0.0-python-3.6 branch May 17, 2024 11:01
MewPurPur pushed a commit to MewPurPur/godot that referenced this pull request Jul 11, 2024
The min SCons version had to be bumped as SCons 3.0 before 3.0.3 seems
broken (see godotengine#92043), and there's little gain from supporting 3.0.3-3.0.5.

3.1.2 is also the first version to avoid ambiguities between Python 2
and Python 3 usage, so we finally use it as the minimum baseline.

Also test against Python 3.6 which is also our minimum supported version.
This should help prevent regressions whenever we modernize the build scripts.
2nafish117 pushed a commit to 2nafish117/godot that referenced this pull request Aug 5, 2024
The min SCons version had to be bumped as SCons 3.0 before 3.0.3 seems
broken (see godotengine#92043), and there's little gain from supporting 3.0.3-3.0.5.

3.1.2 is also the first version to avoid ambiguities between Python 2
and Python 3 usage, so we finally use it as the minimum baseline.

Also test against Python 3.6 which is also our minimum supported version.
This should help prevent regressions whenever we modernize the build scripts.
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.

2 participants