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

Extract and explicitly pass install-scripts to setuptools #530

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Nov 2, 2022

When part of a virtual environment, this option is specifically ignored in configuration files by setuptools, but not on the command line.

This is an actual fix for #518, which wasn't actually fixed by #520 but rather made it far less common.

The test added here doesn't actually create a virtual environment to validate the conditions which motivated this fix, but the test will fail if run under a virtual environment without this fix.

When part of a virtual environment, this option is specifically ignored
in configuration files by setuptools, but not on the command line.
@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Base: 81.66% // Head: 81.48% // Decreases project coverage by -0.17% ⚠️

Coverage data is based on head (30b6be6) compared to base (0bf904e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #530      +/-   ##
==========================================
- Coverage   81.66%   81.48%   -0.18%     
==========================================
  Files          60       58       -2     
  Lines        3566     3375     -191     
  Branches      685      657      -28     
==========================================
- Hits         2912     2750     -162     
+ Misses        602      579      -23     
+ Partials       52       46       -6     
Impacted Files Coverage Δ
colcon_core/task/python/build.py 40.20% <100.00%> (+4.17%) ⬆️
colcon_core/__init__.py
colcon_core/command.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

This looks like a reasonable way to do this to me absent a more reasonable API from setuptools.

colcon_core/task/python/build.py Show resolved Hide resolved
@cottsay cottsay merged commit 84fc2d8 into master Nov 16, 2022
@delete-merged-branch delete-merged-branch bot deleted the cottsay/venvs branch November 16, 2022 23:08
@cottsay cottsay added this to the 0.11.0 milestone Nov 17, 2022
@cottsay cottsay added the bug Something isn't working label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

4 participants