checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.77k stars 561 forks source link

lib: use separate packages for pycriu and crit #2282

Closed rst0git closed 8 months ago

rst0git commented 9 months ago

Newer versions of pip use an isolated virtual environment when building Python projects. However, when the source code of CRIT is copied into the isolated environment, the symlink for ../lib/py (pycriu) becomes invalid. As a workaround, we used the --no-build-isolation option for pip install. However, this functionality has issues in some versions of PIP (https://github.com/pypa/pip/pull/8221, https://github.com/pypa/pip/issues/8165#issuecomment-625401463). To fix this problem, this patch adds separate packages for pycriu and crit, and each package is installed independently.

codecov-commenter commented 9 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (ab73a84) 70.58% compared to head (afd2cf3) 70.54%.

:exclamation: Current head afd2cf3 differs from pull request most recent head d020450. Consider uploading reports for the commit d020450 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2282 +/- ## ============================================ - Coverage 70.58% 70.54% -0.05% ============================================ Files 135 132 -3 Lines 33522 33508 -14 ============================================ - Hits 23663 23638 -25 - Misses 9859 9870 +11 ``` [see 13 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2282/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

adrianreber commented 9 months ago

The most important thing from my point of view is that our DEB and RPM packages still work. The last time we changed something with the way the Python bindings are distributed it required some work to get the packaging effort going again.

I think this needs to be tested with RPM and DEB packaging to ensure this time it works better than last time.

I think the best way to ship the Python bindings would be a separate repository, but the way CRIU uses Python during testing would mean that things get much more complicated if the Python code is another repository.

To make sure RPM packaging works there is a packit service which we could run in CI to ensure there is always a working package.

rst0git commented 9 months ago

Thank you Adrian, enabling buildability checks is a good idea. This pull request aims to fix the error we currently see in CI with CentOS Stream 9 (https://github.com/checkpoint-restore/criu/runs/17505949509).

Should we open a separate pull request for testing RPM and DEB packaging?

avagin commented 9 months ago

Thank you Adrian, enabling buildability checks is a good idea. This pull request aims to fix the error we currently see in CI with CentOS Stream 9 (https://github.com/checkpoint-restore/criu/runs/17505949509).

Do you understand why it fails only on CentOS 9?

rst0git commented 9 months ago

Do you understand why it fails only on CentOS 9?

The problem that causes this error was introduced in pip version 20.1.1[^1] and fixed in version 21.3[^2]. These versions of pip have a legacy behaviour that copies the entire project (crit) directory to a temporary location and break the pycriu symbolic link. This legacy behaviour has been disabled by default and removed in newer versions of pip.

It currently fails on CentOS 9 because the python3-pip package provides version 21.2.3.

[^1]: Revert building of local directories in place, restoring the pre-20.1 behaviour of copying to a temporary directory. (#7555) [^2]: In-tree builds are now the default. --use-feature=in-tree-build is now ignored. --use-deprecated=out-of-tree-build may be used temporarily to ease the transition. (#10495)