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

Fix absolute base python paths conflicting #3325

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

gaborbernat
Copy link
Member

Signed-off-by: Bernát Gábor bgabor8@bloomberg.net

Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
@gaborbernat gaborbernat marked this pull request as ready for review August 13, 2024 16:30
@gaborbernat gaborbernat merged commit 1ee4a33 into tox-dev:main Aug 13, 2024
24 checks passed
@gaborbernat gaborbernat deleted the fix-confl branch August 13, 2024 16:35
@hroncok
Copy link
Contributor

hroncok commented Aug 14, 2024

Hello. Is it expected that this breaks extensions who implement this abstract class?

See fedora-python/tox-current-env#77

@gaborbernat
Copy link
Member Author

yes, sadly I don't have a better option to add new abstract methods here

if spec_base.path is not None:
path = Path(spec_base.path).absolute()
if str(spec_base.path) == sys.executable:
ver, is_64 = sys.version_info, sys.maxsize != 2**32
Copy link
Contributor

Choose a reason for hiding this comment

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

When reviewing fedora-python/tox-current-env#78 @frenzymadness found out that is_64 is always True:

Fedora Rawhide i686

>>> sys.maxsize
2147483647
>>> 2**32
4294967296
>>> sys.maxsize != 2**32
True

Fedora Rawhide x86_64

>>> sys.maxsize
9223372036854775807
>>> 2**32
4294967296
>>> sys.maxsize != 2**32
True

https://docs.python.org/3/library/platform.html#platform.architecture says:

To get at the “64-bitness” of the current interpreter, it is more reliable to query the sys.maxsize attribute:

is_64bits = sys.maxsize > 2**32

Fedora Rawhide i686

>>> sys.maxsize > 2**32
False

Fedora Rawhide x86_64

>>> sys.maxsize > 2**32
True

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good catch both. can you put in a PR to fix it 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I am working on it. I assume you intended a string like cpython3.13-64.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is correct, thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Drafted #3327 -- waiting for the CI to validate it.

path = Path(spec_base.path).absolute()
if str(spec_base.path) == sys.executable:
ver, is_64 = sys.version_info, sys.maxsize != 2**32
spec_base = PythonSpec.from_string_spec(f"{sys.implementation}{ver.major}{ver.minor}-{is_64}")
Copy link
Contributor

@hroncok hroncok Aug 15, 2024

Choose a reason for hiding this comment

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

@frenzymadness also pointed out that the string ends with a boolean, like ...313-True, while according to a PATTERN in virtualenv:

https://github.com/pypa/virtualenv/blob/95c5eed96c225faff01ac2f108368e0c4303989b/src/virtualenv/discovery/py_spec.py#L8

PATTERN = re.compile(r"^(?P<impl>[a-zA-Z]+)?(?P<version>[0-9.]+)?(?:-(?P<arch>32|64))?$")

It should probably end with -32 or -64 if we care about the 32/64bitness.

Moreover, when testing this assumption, I found out that the entire string is probably completely bogus:

>>> ver, is_64 = sys.version_info, sys.maxsize != 2**32
>>> f"{sys.implementation}{ver.major}{ver.minor}-{is_64}"
"namespace(name='cpython', cache_tag='cpython-313', version=sys.version_info(major=3, minor=13, micro=0, releaselevel='candidate', serial=1), hexversion=51183809, _multiarch='x86_64-linux-gnu')313-True"

We probably want to replace sys.implementation with sys.implementation.name if cpython313-64 is what you intended.

naa0yama pushed a commit to naa0yama/PythonBoilerplate that referenced this pull request Aug 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [tox](https://github.com/tox-dev/tox)
([changelog](https://tox.wiki/en/latest/changelog.html)) | `~4.16.0` ->
`~4.18.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/tox/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/tox/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/tox/4.16.0/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/tox/4.16.0/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>tox-dev/tox (tox)</summary>

### [`v4.18.0`](https://github.com/tox-dev/tox/releases/tag/4.18.0)

[Compare
Source](https://github.com/tox-dev/tox/compare/4.17.1...4.18.0)

<!-- Release notes generated using configuration in .github/release.yml
at 4.18.0 -->

#### What's Changed

- Fix [#&#8203;3278](https://github.com/tox-dev/tox/issues/3278) -
Boost temporary directories cleanup in tests by
[@&#8203;ziima](https://github.com/ziima) in
[tox-dev/tox#3323
- Fix absolute base python paths conflicting by
[@&#8203;gaborbernat](https://github.com/gaborbernat) in
[tox-dev/tox#3325
- Fix [#&#8203;3318](https://github.com/tox-dev/tox/issues/3318) -
Suppress spinner in parallel runs in CI by
[@&#8203;ziima](https://github.com/ziima) in
[tox-dev/tox#3321

**Full Changelog**:
tox-dev/tox@4.17.1...4.18.0

### [`v4.17.1`](https://github.com/tox-dev/tox/releases/tag/4.17.1)

[Compare
Source](https://github.com/tox-dev/tox/compare/4.17.0...4.17.1)

<!-- Release notes generated using configuration in .github/release.yml
at 4.17.1 -->

##### What's Changed

- Restore limited `<major>.<minor>` environment name support by
[@&#8203;gaborbernat](https://github.com/gaborbernat) in
[tox-dev/tox#3319
- fix(tox_env.python): do not process absolute paths to interpreter as
PythonSpec by [@&#8203;paveldikov](https://github.com/paveldikov) in
[tox-dev/tox#3311

##### New Contributors

- [@&#8203;paveldikov](https://github.com/paveldikov) made their first
contribution in
[tox-dev/tox#3311

**Full Changelog**:
tox-dev/tox@4.17.0...4.17.1

### [`v4.17.0`](https://github.com/tox-dev/tox/releases/tag/4.17.0)

[Compare
Source](https://github.com/tox-dev/tox/compare/4.16.0...4.17.0)

<!-- Release notes generated using configuration in .github/release.yml
at 4.17.0 -->

##### What's Changed

- Fix user guide system overview so nodes don't overlap. by
[@&#8203;Tom01098](https://github.com/Tom01098) in
[tox-dev/tox#3307
- Table with list of default env vars per OS by
[@&#8203;seyidaniels](https://github.com/seyidaniels) in
[tox-dev/tox#3291
- Add GraalPy and test both GraalPy and Jython env identifiers by
[@&#8203;timfel](https://github.com/timfel) in
[tox-dev/tox#3312
- Add on platform constat to core by
[@&#8203;gaborbernat](https://github.com/gaborbernat) in
[tox-dev/tox#3315

##### New Contributors

- [@&#8203;Tom01098](https://github.com/Tom01098) made their first
contribution in
[tox-dev/tox#3307
- [@&#8203;timfel](https://github.com/timfel) made their first
contribution in
[tox-dev/tox#3312

**Full Changelog**:
tox-dev/tox@4.16.0...4.17.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/naa0yama/PythonBoilerplate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM4LjI2LjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbImRhdGFzb3VyY2U6cHlwaSIsImRlcFR5cGU6ZGV2IiwibWFuYWdlcjpwb2V0cnkiLCJyZW5vdmF0ZSIsInVwZGF0ZS1taW5vciIsInZlcnNpb25pbmc6cG9ldHJ5Il19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
hofbi pushed a commit to luminartech/dev-tools that referenced this pull request Sep 3, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [tox](https://github.com/tox-dev/tox)
([changelog](https://tox.wiki/en/latest/changelog.html)) | `==4.17.1` ->
`==4.18.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/tox/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/tox/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/tox/4.17.1/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/tox/4.17.1/4.18.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>tox-dev/tox (tox)</summary>

### [`v4.18.0`](https://github.com/tox-dev/tox/releases/tag/4.18.0)

[Compare
Source](https://github.com/tox-dev/tox/compare/4.17.1...4.18.0)

<!-- Release notes generated using configuration in .github/release.yml
at 4.18.0 -->

#### What's Changed

- Fix [#&#8203;3278](https://github.com/tox-dev/tox/issues/3278) -
Boost temporary directories cleanup in tests by
[@&#8203;ziima](https://github.com/ziima) in
[tox-dev/tox#3323
- Fix absolute base python paths conflicting by
[@&#8203;gaborbernat](https://github.com/gaborbernat) in
[tox-dev/tox#3325
- Fix [#&#8203;3318](https://github.com/tox-dev/tox/issues/3318) -
Suppress spinner in parallel runs in CI by
[@&#8203;ziima](https://github.com/ziima) in
[tox-dev/tox#3321

**Full Changelog**:
tox-dev/tox@4.17.1...4.18.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am on the first day of the
month" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/luminartech/dev-tools).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41Ni4wIiwidXBkYXRlZEluVmVyIjoiMzguNTYuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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