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.

Sign up for GitHub

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

Jump to bottom

Strict dependencies allows binary packages for indirect dependencies #1473

Closed
carlcsaposs-canonical opened this issue Jan 10, 2024 · 9 comments
Closed

Strict dependencies allows binary packages for indirect dependencies #1473

carlcsaposs-canonical opened this issue Jan 10, 2024 · 9 comments
Assignees
@lengau
Labels
Bug Something isn't working triaged

Comments

@carlcsaposs-canonical
Copy link
Contributor

Bug Description

charm-strict-dependencies: true changes the behavior of charm-binary-python-packages (source)

If requirements.txt does not contain all dependencies (e.g. it contains ops but not pyyaml, which is a dependency of ops), indirect dependencies (e.g. pyyaml) will be installed from binary wheels instead of from source—even if they are not specified in charm-binary-python-packages

This is because charmcraft uses pip install --no-binary with a list of package names in requirements.txt

To solve this, charmcraft should use pip install --no-binary :all: --only-binary with a list of package names in charm-binary-python-packages (allowlist binary packages instead of blocklist source packages, since not all packages are necessarily in requirements.txt). Note that order matters

To Reproduce

python3 -m venv venv3
source venv3/bin/activate
pip install ops --no-binary ops

Environment

N/A

charmcraft.yaml

N/A

Relevant log output

ubuntu@j31-3:~$ python3 -m venv venv3
ubuntu@j31-3:~$ source venv3/bin/activate
(venv3) ubuntu@j31-3:~$ pip install ops --no-binary ops
Collecting ops
  Downloading ops-2.9.0.tar.gz (255 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 255.8/255.8 KB 3.1 MB/s eta 0:00:00
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting PyYAML==6.*
  Downloading PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (705 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 705.5/705.5 KB 38.4 MB/s eta 0:00:00
Collecting websocket-client==1.*
  Downloading websocket_client-1.7.0-py3-none-any.whl (58 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 58.5/58.5 KB 11.2 MB/s eta 0:00:00
Building wheels for collected packages: ops
  Building wheel for ops (pyproject.toml) ... done
  Created wheel for ops: filename=ops-2.9.0-py3-none-any.whl size=153069 sha256=4962af468d3e4794aab1fe44b2c69890d9b3adc4e3d9e1b154dd8bbbb212f4d4
  Stored in directory: /home/ubuntu/.cache/pip/wheels/a9/87/9f/a3316be80c2829313331b849dd89a0f5eff5673a67a52b386d
Successfully built ops
Installing collected packages: websocket-client, PyYAML, ops
Successfully installed PyYAML-6.0.1 ops-2.9.0 websocket-client-1.7.0
(venv3) ubuntu@j31-3:~$ deactivate 
ubuntu@j31-3:~$ python3 -m venv venv4
ubuntu@j31-3:~$ source venv4/bin/activate
(venv4) ubuntu@j31-3:~$ pip install ops --no-binary :all: --only-binary websocket-client
Collecting ops
  Using cached ops-2.9.0.tar.gz (255 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting websocket-client==1.*
  Using cached websocket_client-1.7.0-py3-none-any.whl (58 kB)
Collecting PyYAML==6.*
  Using cached PyYAML-6.0.1.tar.gz (125 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: ops, PyYAML
  Building wheel for ops (pyproject.toml) ... done
  Created wheel for ops: filename=ops-2.9.0-py3-none-any.whl size=153069 sha256=1a128b785e34dceab8d3b7c1a67e00f81e325cca5828b4ee84d87c880e272f86
  Stored in directory: /home/ubuntu/.cache/pip/wheels/a9/87/9f/a3316be80c2829313331b849dd89a0f5eff5673a67a52b386d
  Building wheel for PyYAML (pyproject.toml) ... done
  Created wheel for PyYAML: filename=PyYAML-6.0.1-cp310-cp310-linux_x86_64.whl size=45360 sha256=112934fbc8667a9e61883db5bff2cab57f3fbda7e15741578df5c98be8100ae3
  Stored in directory: /home/ubuntu/.cache/pip/wheels/31/d1/78/088142cfc0422adcca7bb0af40ef696b55b777a44f2e10739c
Successfully built ops PyYAML
Installing collected packages: websocket-client, PyYAML, ops
Successfully installed PyYAML-6.0.1 ops-2.9.0 websocket-client-1.7.0
(venv4) ubuntu@j31-3:~$ deactivate 
ubuntu@j31-3:~$ python3 -m venv venv5
ubuntu@j31-3:~$ source venv5/bin/activate
(venv5) ubuntu@j31-3:~$ pip install ops --only-binary websocket-client --no-binary :all:
Collecting ops
  Using cached ops-2.9.0.tar.gz (255 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting PyYAML==6.*
  Using cached PyYAML-6.0.1.tar.gz (125 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Collecting websocket-client==1.*
  Downloading websocket-client-1.7.0.tar.gz (54 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 54.0/54.0 KB 1.9 MB/s eta 0:00:00
  Preparing metadata (setup.py) ... done
Skipping wheel build for websocket-client, due to binaries being disabled for it.
Building wheels for collected packages: ops, PyYAML
  Building wheel for ops (pyproject.toml) ... done
  Created wheel for ops: filename=ops-2.9.0-py3-none-any.whl size=153069 sha256=9b6992b436b4be1ea685e53a257e3aef43410d259d08f584855e7b2bbef74946
  Stored in directory: /home/ubuntu/.cache/pip/wheels/a9/87/9f/a3316be80c2829313331b849dd89a0f5eff5673a67a52b386d
  Building wheel for PyYAML (pyproject.toml) ... done
  Created wheel for PyYAML: filename=PyYAML-6.0.1-cp310-cp310-linux_x86_64.whl size=45360 sha256=c25a49eb3e7e60d13a6424662d8087ea7ec5c7cbe740e0cab842c7bad7e58fdf
  Stored in directory: /home/ubuntu/.cache/pip/wheels/31/d1/78/088142cfc0422adcca7bb0af40ef696b55b777a44f2e10739c
Successfully built ops PyYAML
Installing collected packages: websocket-client, PyYAML, ops
  Running setup.py install for websocket-client ... done
Successfully installed PyYAML-6.0.1 ops-2.9.0 websocket-client-1.7.0
@carlcsaposs-canonical carlcsaposs-canonical added the Bug Something isn't working label Jan 10, 2024
@dimaqq
Copy link
Contributor

dimaqq commented Jun 21, 2024

I've also hit this, asked at https://matrix.to/#/!LhFxJIPEcCacgdMghH:ubuntu.com/$gI6OSzDnA3a3khHCUOU-xGPENRQEs8eUs1YJRQL-N3g?via=ubuntu.com&via=matrix.org&via=lgp.sh

@syncronize-issues-to-jira Syncronize issues to Jira
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/CRAFT-3055.

This message was autogenerated

@lengau lengau self-assigned this Jul 15, 2024
@lengau
Copy link
Collaborator

lengau commented Jul 15, 2024

The fix to this should be to use --no-dependencies in the pip line, which may break existing charms. Using --only-binary isn't feasible because we need to fall back to building from source if a binary isn't available.

My current strategy is put this into 3.1 (since 3.0 never hit stable) and just put a warning in 2.x. However I'm interested to hear from @carlcsaposs-canonical and @dimaqq since you're two of the biggest users strict mode.

@carlcsaposs-canonical
Copy link
Contributor Author

we need to fall back to building from source if a binary isn't available

why have different behavior for direct and indirect dependencies on strict mode? (my understanding is that there will be no fallback for direct dependencies)

If it's not necessary to fall back to building from source for direct deps, I don't understand why it's needed for indirect deps. Since charmcraft is a build tool (not a runtime tool), failing a build and instructing the user to add something to charm-binary-python-packages, IMO, isn't a big deal—silently adding a binary dependency, IMO, is a big deal

@lengau
Copy link
Collaborator

lengau commented Jul 16, 2024

Direct dependencies fall back to being able to build from source if there's no binary available. charm-binary-python-packages only tells Charmcraft to allow those specific packages from binaries. The problem is, pip doesn't have a --allow-binary type option, so we can't say --no-binary=:all: --allow-binary=pkg1,pkg2.

You bring up a good point about what's a big or small deal. I'll have to consider it.

@carlcsaposs-canonical
Copy link
Contributor Author

Direct dependencies fall back to being able to build from source if there's no binary available.

are you sure? it looks like direct dependencies use --no-binary
e.g. https://github.com/canonical/mysql-router-k8s-operator/actions/runs/9966570525/job/27538859945#step:12:302

@lengau
Copy link
Collaborator

lengau commented Jul 18, 2024

I mean for ones that have binaries allowed. We don't enforce using binaries for anything - charm-binary-python-packages just removes them from the --no-binary list. But I had to make that list in order to allow charm-binary-python-packages to work in strict mode.

I suppose a backwards-compatible change for 2.x would be that if you're not using charm-binary-python-packages with strict mode it simply sets --no-binary=:all:. It's not a complete fix, but it's something that I could implement in both anyway.

@lengau lengau mentioned this issue Jul 18, 2024
Merged
lengau added a commit that referenced this issue Jul 18, 2024
@lengau
fix(pip): set --no-binary=:all: if possible (#1740)
55434e0 
This reduces the chances of having pip install binary packages as
indirect dependencies.

Partial fix for #1473
@carlcsaposs-canonical
Copy link
Contributor Author

oh I see

I think --no-dependencies in strict mode makes sense then

lengau added a commit that referenced this issue Jul 19, 2024
@lengau
fix(pip): set --no-binary=:all: if possible (#1740)
bfa845b 
This reduces the chances of having pip install binary packages as
indirect dependencies.

Partial fix for #1473
lengau added a commit that referenced this issue Jul 19, 2024
@lengau
fix(pip): set --no-binary=:all: if possible (#1740)
2aa2da7 
This reduces the chances of having pip install binary packages as
indirect dependencies.

Partial fix for #1473
lengau added a commit that referenced this issue Jul 19, 2024
@lengau
fix(pip): set --no-binary=:all: if possible (#1740)
889254f 
This reduces the chances of having pip install binary packages as
indirect dependencies.

Partial fix for #1473
@lengau lengau mentioned this issue Aug 7, 2024
Closed
@lengau lengau closed this as completed Aug 7, 2024
@carlcsaposs-canonical
Copy link
Contributor Author

@lengau was this partially fixed or fully fixed? looking at the linked commits, I'm a bit confused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lengau lengau

Labels
Bug Something isn't working triaged
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dimaqq @sergiusens @lengau @carlcsaposs-canonical