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

build: python 3 compatibility #7723

Merged
merged 1 commit into from Mar 29, 2016
Merged

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Mar 20, 2016

Ubuntu 16.04 "xenial xerus" does not come with Python 2.x by default. It is possible to install a python-2.7 package, but this has its own problem: no python or python2 symlink (see #7717).

This fixes the following scripts to work with python 3:

  • make check (bctest,py, bitcoin-util-test.py)
  • make translate (extract_strings_qt.py)
  • make symbols-check (symbol-check.py)
  • make security-check (security-check.py)
  • make deploy for OS X (custom_dsstore.py, macdeployqtplus)

Explicitly call the python commands using $(PYTHON) (detected by autoconf) instead of relying on the interpreter line at the top of the scripts.

Python 2.x compatibility should be unaffected. For the build system I think it's good to have both Python2 and Python3 compatibility. This is not necessary for the other python scripts, such as the RPC tests. For practical reasons it's ok to have an explicit Python 2.7 dependency for those - though we'll have to document that.

@laanwj laanwj force-pushed the 2016_03_build_python_3 branch 2 times, most recently from 076dab8 to 40a9d86 Compare March 20, 2016 18:03
@laanwj
Copy link
Member Author

laanwj commented Mar 21, 2016

Ping @theuni

@maflcko
Copy link
Member

maflcko commented Mar 21, 2016

Looks good. Concept ACK 40a9d86

@jonasschnelli
Copy link
Contributor

Concept ACK.

Why not use !/usr/bin/env python as interpreter?
IMO we should at least remove /usr/bin/python2 and use /usr/bin/python (https://github.com/bitcoin/bitcoin/pull/7723/files#diff-d403511be890783565b1e2d164beec8aR1).

@laanwj
Copy link
Member Author

laanwj commented Mar 21, 2016

Why not use !/usr/bin/env python as interpreter?

Interpeter shouldn't matter anymore because all usages invoke the explicitly located python executable.
(could even remove it, in principle)

@theuni
Copy link
Member

theuni commented Mar 22, 2016

@laanwj Aha, clever fix :)

I'm not well-versed in python2/python3 to comment on the compat there, but concept ack for sure.

Looks like some of the mac deploy scripts are missing, though:

contrib/macdeploy/custom_dsstore.py
contrib/macdeploy/macdeployqtplus

Those both get hit via make deploy

@laanwj
Copy link
Member Author

laanwj commented Mar 23, 2016

contrib/macdeploy/custom_dsstore.py
contrib/macdeploy/macdeployqtplus

Right, good catch, will have a look at those.

@laanwj
Copy link
Member Author

laanwj commented Mar 23, 2016

@cfields see b40e365c2a24019e2b3e2651510874d0cecf2ae3
Works in python2 at least, haven't tested w/ python3 yet, I don't have a macosx dev environment set up.

@laanwj
Copy link
Member Author

laanwj commented Mar 23, 2016

Works in python2 at least, haven't tested w/ python3 yet, I don't have a macosx dev environment set up.

Ok, tested by temporarily changing the detection to only 'see' python3. It passed.

@@ -575,7 +575,7 @@ if len(config.fancy) == 1:
if fancy.has_key("background_picture"):
Copy link
Member

Choose a reason for hiding this comment

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

Nit: there is no has_key in py3

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

@maflcko
Copy link
Member

maflcko commented Mar 28, 2016

utACK b40e365

Ubuntu 16.04 "xenial xerus" does not come with Python 2.x by default.
It is possible to install a python-2.7 package, but this has its own
problem: no `python` or `python2` symlink (see bitcoin#7717).

This fixes the following scripts to work with python 3:
- `make check` (bctest,py, bitcoin-util-test.py)
- `make translate` (extract_strings_qt.py)
- `make symbols-check` (symbol-check.py)
- `make security-check` (security-check.py)

Explicitly call the python commands using $(PYTHON) instead
of relying on the interpreter line at the top of the scripts.
@laanwj
Copy link
Member Author

laanwj commented Mar 29, 2016

Squashed b875273, b40e365 and cb01698 into 18f05c7

@laanwj laanwj merged commit 18f05c7 into bitcoin:master Mar 29, 2016
laanwj added a commit that referenced this pull request Mar 29, 2016
18f05c7 build: python 3 compatibility (Wladimir J. van der Laan)
laanwj added a commit to laanwj/bitcoin that referenced this pull request Apr 3, 2016
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
theuni pushed a commit to theuni/bitcoin that referenced this pull request Apr 21, 2016
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
pstratem pushed a commit to pstratem/bitcoin that referenced this pull request May 1, 2016
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
ChoHag added a commit to ChoHag/bitcoin that referenced this pull request Jun 28, 2016
maflcko pushed a commit that referenced this pull request Aug 15, 2016
7b01ce2 Favour python over python2 as per PR #7723 (Matthew King)
873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017
18f05c7 build: python 3 compatibility (Wladimir J. van der Laan)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 20, 2017
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
codablock pushed a commit to codablock/dash that referenced this pull request Jan 8, 2018
…n/env)

7b01ce2 Favour python over python2 as per PR bitcoin#7723 (Matthew King)
873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 12, 2018
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
michelvankessel pushed a commit to michelvankessel/blackcoin-more that referenced this pull request Jan 4, 2019
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
…n/env)

7b01ce2 Favour python over python2 as per PR bitcoin#7723 (Matthew King)
873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
This fixes the gitian MacOSX build, it was broken in bitcoin#7723.

The patch to `native_mac_alias` should probably make it upstream.
CryptoCentric added a commit to absolute-community/absolute that referenced this pull request Feb 15, 2019
Tests: Use portable #! in python scripts

(/usr/bin/env)

7b01ce2 Favour python over python2 as per PR bitcoin#7723 (Matthew King)
873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants