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

snap: prepare override scripts to allow rebuilding #2223

Merged
merged 2 commits into from Aug 27, 2018

Conversation

sergiusens
Copy link
Collaborator

@sergiusens sergiusens commented Aug 23, 2018

This projects snapcraft.yaml does not allow for easy rebuilding
given the use of linking and patching to make ctypes work properly
and the required patches to PyYAML.

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
  • If this is a new feature. Have you discussed the design on the forum?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

This projects snapcraft.yaml does not allow for easy rebuilding
given the use of linking and patching to make ctypes work properly
and the required patches to PyYAML.

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Copy link
Contributor

@kyrofa kyrofa left a comment

Choose a reason for hiding this comment

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

This seems alright, although I have a few suggestions.

@@ -54,9 +54,15 @@ parts:
snapcraftctl build
TRIPLET_PATH="$SNAPCRAFT_PART_INSTALL/usr/lib/$(gcc -print-multiarch)"
LIBSODIUM=$(readlink -n $TRIPLET_PATH/libsodium.so.18)
# Remove so the link can be recreated on re-builds
[ -f "$TRIPLET_PATH/libsodium.so" ] && rm "$TRIPLET_PATH/libsodium.so"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just rm -f "$TRIPLET_PATH/libsodium.so" is simpler and results in the same thing.

patch -d $SNAPCRAFT_PART_INSTALL/lib/python3.5/site-packages -p1 < $SNAPCRAFT_STAGE/pyyaml-support-high-codepoints.diff
patch $SNAPCRAFT_PART_INSTALL/usr/lib/python3.5/ctypes/__init__.py $SNAPCRAFT_STAGE/ctypes_init.diff
# These checks are ugly but allow us to rebuild.
[ -f $SNAPCRAFT_PART_INSTALL/lib/python3.5/site-packages/yaml/emitter.py.orig ] && mv $SNAPCRAFT_PART_INSTALL/lib/python3.5/site-packages/yaml/emitter.py.orig $SNAPCRAFT_PART_INSTALL/lib/python3.5/site-packages/yaml/emitter.py
Copy link
Contributor

@kyrofa kyrofa Aug 23, 2018

Choose a reason for hiding this comment

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

I see no stage or prime filter to remove these, nor do I see logic in override-prime to remove them-- probably best not to ship .orig files in the snap, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought it would be rather harmless, but sure.

@codecov-io
Copy link

codecov-io commented Aug 23, 2018

Codecov Report

Merging #2223 into master will decrease coverage by 0.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2223      +/-   ##
==========================================
- Coverage   91.09%   90.66%   -0.44%     
==========================================
  Files         211      212       +1     
  Lines       13466    13453      -13     
  Branches     2008     2000       -8     
==========================================
- Hits        12267    12197      -70     
- Misses        816      861      +45     
- Partials      383      395      +12
Impacted Files Coverage Δ
snapcraft/internal/lifecycle/_containers.py 60.46% <0%> (-22.47%) ⬇️
snapcraft/internal/lxd/_project.py 78.18% <0%> (-13.89%) ⬇️
snapcraft/internal/lxd/_cleanbuilder.py 94.28% <0%> (-5.72%) ⬇️
snapcraft/internal/lxd/_containerbuild.py 89.38% <0%> (-5.27%) ⬇️
snapcraft/project/_project_options.py 85.6% <0%> (-2.28%) ⬇️
snapcraft/internal/pluginhandler/_plugin_loader.py 91.96% <0%> (-1.79%) ⬇️
snapcraft/internal/build_providers/errors.py 98.68% <0%> (-1.32%) ⬇️
snapcraft/internal/repo/snaps.py 89.82% <0%> (-1.2%) ⬇️
snapcraft/cli/parts.py 81.48% <0%> (-0.67%) ⬇️
snapcraft/cli/containers.py 92.85% <0%> (-0.48%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8acd203...fe8c0ab. Read the comment docs.

patch -b "$PYTHON_PACKAGE_PATH/ctypes/__init__.py" patches/ctypes_init.diff
patch -b -d "$SITE_PACKAGES_PATH/" -p1 < patches/pyyaml-support-high-codepoints.diff

# Save patches to allow rebuilding
Copy link
Contributor

Choose a reason for hiding this comment

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

Clever, took me a second to figure out what you were doing here.

@sergiusens
Copy link
Collaborator Author

@sergiusens sergiusens merged commit ebbdd7d into canonical:master Aug 27, 2018
@sergiusens sergiusens deleted the snap-rebuild branch August 27, 2018 12:58
xnox pushed a commit to xnox/snapcraft that referenced this pull request Jul 3, 2020
This projects snapcraft.yaml does not allow for easy rebuilding
given the use of linking and patching to make ctypes work properly
and the required patches to PyYAML.

LP: #1789206

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants