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

snapcraft: build and use patchelf 0.18 #14908

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bboozzoo
Copy link
Contributor

@bboozzoo bboozzoo commented Jan 8, 2025

Patching with patchelf 0.14.3 available in 22.04 produces the result binaries get silently broken and break parsing with python-elfutils.

Inspectign a patched binary with readelf also shows a warning like so:

snapcraft-snapd-on-amd64-for-amd64-1579591 ../project# readelf -h snapd.orig
ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
  Class:                             ELF64
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Position-Independent Executable file)
  Machine:                           Advanced Micro Devices X86-64
  Version:                           0x1
  Entry point address:               0x4800e0
  Start of program headers:          64 (bytes into file)
  Start of section headers:          680 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           56 (bytes)
  Number of program headers:         12
  Size of section headers:           64 (bytes)
  Number of section headers:         37
  Section header string table index: 35
readelf: Warning: Section 0 has an out of range sh_link value of 72

The upstream patchelf 0.18 is fixed, so do a local build patchelf and use it instead of a distro package.

Related: SNAPDENG-34352

Will need a rebase once #14901 lands

@bboozzoo bboozzoo requested review from valentindavid and zyga January 8, 2025 13:25
Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

I think the intent is good but I left a remark about source code and patches.

- g++
- pkg-config
- wget
source: https://github.com/NixOS/patchelf/releases/download/0.18.0/patchelf-0.18.0.tar.bz2
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the Ubuntu or Debian archive here?

http://archive.ubuntu.com/ubuntu/pool/universe/p/patchelf/patchelf_0.18.0.orig.tar.gz

Debian applies two patches that we should consider as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@bboozzoo bboozzoo force-pushed the bboozzoo/patchelf-fix branch from 7bd74d2 to d341191 Compare January 9, 2025 13:40
@bboozzoo bboozzoo requested a review from zyga January 9, 2025 13:40
@bboozzoo bboozzoo force-pushed the bboozzoo/patchelf-fix branch from d341191 to c2a58bf Compare January 10, 2025 07:27
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.22%. Comparing base (24a0034) to head (77f4c9e).
Report is 96 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14908      +/-   ##
==========================================
+ Coverage   78.20%   78.22%   +0.02%     
==========================================
  Files        1151     1157       +6     
  Lines      151396   152941    +1545     
==========================================
+ Hits       118402   119642    +1240     
- Misses      25662    25912     +250     
- Partials     7332     7387      +55     
Flag Coverage Δ
unittests 78.22% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bboozzoo bboozzoo force-pushed the bboozzoo/patchelf-fix branch from c2a58bf to d13b413 Compare January 10, 2025 08:15
Copy link
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link
Contributor

@valentindavid valentindavid left a comment

Choose a reason for hiding this comment

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

Thank you.

# as of 0.18.0-1.1build1 in Oracular, here are no relevant patches to apply
source: http://archive.ubuntu.com/ubuntu/pool/universe/p/patchelf/patchelf_0.18.0.orig.tar.gz
source-checksum: sha256/1451d01ee3a21100340aed867d0b799f46f0b1749680028d38c3f5d0128fb8a7
override-prime: |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can do prime: [-*] also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, applied

Patching with patchelf 0.14.3 available in 22.04 produces the result
binaries get silently broken and break parsing with python-elfutils.

Inspectign a patched binary with readelf also shows a warning like so:

    snapcraft-snapd-on-amd64-for-amd64-1579591 ../project# readelf -h snapd.orig
    ELF Header:
      Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
      Class:                             ELF64
      Data:                              2's complement, little endian
      Version:                           1 (current)
      OS/ABI:                            UNIX - System V
      ABI Version:                       0
      Type:                              DYN (Position-Independent Executable file)
      Machine:                           Advanced Micro Devices X86-64
      Version:                           0x1
      Entry point address:               0x4800e0
      Start of program headers:          64 (bytes into file)
      Start of section headers:          680 (bytes into file)
      Flags:                             0x0
      Size of this header:               64 (bytes)
      Size of program headers:           56 (bytes)
      Number of program headers:         12
      Size of section headers:           64 (bytes)
      Number of section headers:         37
      Section header string table index: 35
    readelf: Warning: Section 0 has an out of range sh_link value of 72

The upstream patchelf 0.18 is fixed, so do a local build patchelf and
use it instead of a distro package.

Signed-off-by: Maciej Borzecki <[email protected]>
@bboozzoo bboozzoo force-pushed the bboozzoo/patchelf-fix branch from d13b413 to 77f4c9e Compare January 10, 2025 12:50
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.

3 participants