You are not logged in.

#1 2021-10-25 11:13:47

fedev
Member
Registered: 2017-09-21
Posts: 12

Request - PKGBUILD Review

Hello!

I'm looking into submitting a PKGBUILD file to AUR and the Arch Wiki suggests I should be asking for someone to help review the files here (perhaps suggest a name for the package as well). It is a patched version of glibc for arm7h that enables to use the latest version of widevine. Discussion on why this is needed can be found here.

It is based on the PKGBUILD file which can be found here.

The following files are the ones that differ from the original:

glibc-add-support-for-SHT_RELR-sections.patch: http://ix.io/3CMq
glibc-tls-libwidevinecdm.so-since-4.10.2252.0-has-TLS-with.patch: http://ix.io/3CMr
PKGBUILD file (TODO: update maintainer): http://ix.io/3CMl

Edit: Also, should I change the PKGBUILD file to unset the CFLAGS as mentioned here? It doesn't compile otherwise with the shipped makepkg.conf file.


Thanks!

Last edited by fedev (2021-10-25 11:32:08)

Offline

#2 2021-10-25 13:25:20

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

To give a high-level explanation why one would need such a package: Without this patch it is not possible to view DRMed content with Kodi on arm.
I'm unsure if it is possible to view DRM content in Chrome or Firefox without this patch (I've currently no environment for this to test), but I think not.

So this would be a nice way to workaround the issue until patches to glibc land upstream (which I assume will be measured in months not weeks..).
And it would be an "opt in" by the user.

Last edited by pejobo (2021-10-25 20:31:57)

Offline

#3 2021-10-26 11:38:17

Lone_Wolf
Member
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 9,596

Re: Request - PKGBUILD Review

fedev wrote:

Edit: Also, should I change the PKGBUILD file to unset the CFLAGS as mentioned here? It doesn't compile otherwise with the shipped makepkg.conf file.

There's a better option for that by using options= , check man PKGBUILD .

Is this only needed for armv7 architecture ?

Last edited by Lone_Wolf (2021-10-26 11:38:34)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
Did you use the guided installer ? If yes, I can't help you.

(A works at time B)  && (time C > time B ) ≠  (A works at time C)

Offline

#4 2021-10-26 20:21:44

loqs
Member
Registered: 2014-03-06
Posts: 14,282

Re: Request - PKGBUILD Review

Have you tried using the commit(s) from https://sourceware.org/git/?p=glibc.git … skray/relr that is being considered for merger into glibc?
options+=(!buildflags) would unset CPPFLAGS, CFLAGS,  CXXFLAGS, LDFLAGS removing all the standard hardening and optimization flags.  I would suggest trying to locate which option in CFLAGS is causing the issue and remove that from the string.

Offline

#5 2021-10-27 13:16:05

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

Concerning the buildflags: The PKGFILE for glibc of archlinuxarm.org is hosted here: https://github.com/archlinuxarm/PKGBUIL … core/glibc

Offline

#6 2021-11-09 15:10:05

fedev
Member
Registered: 2017-09-21
Posts: 12

Re: Request - PKGBUILD Review

loqs wrote:

Have you tried using the commit(s) from https://sourceware.org/git/?p=glibc.git … skray/relr that is being considered for merger into glibc?
options+=(!buildflags) would unset CPPFLAGS, CFLAGS,  CXXFLAGS, LDFLAGS removing all the standard hardening and optimization flags.  I would suggest trying to locate which option in CFLAGS is causing the issue and remove that from the string.

The section that I need to remove in makepkg.conf to get this to compile is:

-Wp,-D_FORTIFY_SOURCE=2

So I guess that's the one.

How should I go about it?

Offline

#7 2021-11-09 15:16:38

loqs
Member
Registered: 2014-03-06
Posts: 14,282

Re: Request - PKGBUILD Review

  CFLAGS=${CFLAGS/-Wp,-D_FORTIFY_SOURCE=2/}
  CXXFLAGS=${CXXFLAGS/-Wp,-D_FORTIFY_SOURCE=2/}

Offline

#8 2021-11-15 10:02:41

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

Hi @fedev,

First of all, thanks for setting this up. I tried following your instructions, but couldn't get it to work. I had to modify the md5 for sdt.h and got the following error at the end of the build:

In file included from dynamic-link.h:99,
                 from dl-load.c:60:
get-dynamic-info.h: In function 'elf_get_dynamic_info':
get-dynamic-info.h:107:24: error: 'DT_RELR' undeclared (first use in this function); did you mean 'DT_RELA'?
  107 |       ADJUST_DYN_INFO (DT_RELR);
      |                        ^~~~~~~
get-dynamic-info.h:83:11: note: in definition of macro 'ADJUST_DYN_INFO'
   83 |  if (info[tag] != NULL)            \
      |           ^~~
get-dynamic-info.h:107:24: note: each undeclared identifier is reported only once for each function it appears in
  107 |       ADJUST_DYN_INFO (DT_RELR);
      |                        ^~~~~~~
get-dynamic-info.h:83:11: note: in definition of macro 'ADJUST_DYN_INFO'
   83 |  if (info[tag] != NULL)            \
      |           ^~~
In file included from ../include/assert.h:1,
                 from get-dynamic-info.h:22,
                 from dynamic-link.h:99,
                 from dl-load.c:60:
get-dynamic-info.h:137:18: error: 'DT_RELRENT' undeclared (first use in this function); did you mean 'DT_RELAENT'?
  137 |     assert (info[DT_RELRENT]->d_un.d_val == sizeof (ElfW(Relr)));
      |                  ^~~~~~~~~~
../assert/assert.h:105:20: note: in definition of macro 'assert'
  105 |   ((void) sizeof ((expr) ? 1 : 0), __extension__ ({   \
      |                    ^~~~
../elf/link.h:30:27: error: 'Elf32_Relr' undeclared (first use in this function); did you mean 'Elf32_Rela'?
   30 | #define ElfW(type) _ElfW (Elf, __ELF_NATIVE_CLASS, type)
      |                           ^~~
../assert/assert.h:105:20: note: in definition of macro 'assert'
  105 |   ((void) sizeof ((expr) ? 1 : 0), __extension__ ({   \
      |                    ^~~~
../elf/link.h:31:22: note: in expansion of macro '_ElfW_1'
   31 | #define _ElfW(e,w,t) _ElfW_1 (e, w, _##t)
      |                      ^~~~~~~
../elf/link.h:30:20: note: in expansion of macro '_ElfW'
   30 | #define ElfW(type) _ElfW (Elf, __ELF_NATIVE_CLASS, type)
      |                    ^~~~~
get-dynamic-info.h:137:53: note: in expansion of macro 'ElfW'
  137 |     assert (info[DT_RELRENT]->d_un.d_val == sizeof (ElfW(Relr)));
      |                                                     ^~~~
../assert/assert.h:105:34: warning: left-hand operand of comma expression has no effect [-Wunused-value]
  105 |   ((void) sizeof ((expr) ? 1 : 0), __extension__ ({   \
      |                                  ^
get-dynamic-info.h:137:5: note: in expansion of macro 'assert'
  137 |     assert (info[DT_RELRENT]->d_un.d_val == sizeof (ElfW(Relr)));
      |     ^~~~~~
make[2]: *** [../o-iterator.mk:9: /home/alarm/builds/glibc/src/glibc-build/elf/dl-load.o] Error 1
make[2]: Leaving directory '/home/alarm/builds/glibc/src/glibc-2.33/elf'
make[1]: *** [Makefile:479: elf/subdir_lib] Error 2
make[1]: Leaving directory '/home/alarm/builds/glibc/src/glibc-2.33'
make: *** [Makefile:9: all] Error 2
==> ERROR: A failure occurred in build().
    Aborting...

Any pointers on where I went wrong?

Offline

#9 2021-11-15 13:29:54

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

Replying to my own question: There was a blank line too much in the `elf.h` section of glibc-add-support-for-SHT_RELR-sections.patch which apparently confused patch somehow.

 #define SHT_GNU_HASH	  0x6ffffff6	/* GNU-style hash table.  */

@@ -664,6 +665,12 @@ typedef struct
   Elf64_Sxword	r_addend;		/* Addend */

Needs to be

 #define SHT_GNU_HASH	  0x6ffffff6	/* GNU-style hash table.  */
@@ -664,6 +665,12 @@ typedef struct
   Elf64_Sxword	r_addend;		/* Addend */

And then it compiled.

Offline

#10 2021-11-15 13:34:14

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

Found a typo in the PKGBUILD, which makes it impossible to install this as is.

conflics=("glibc")

should be

conflicts=("glibc")

Offline

#11 2021-11-15 13:37:17

fedev
Member
Registered: 2017-09-21
Posts: 12

Re: Request - PKGBUILD Review

ErwinJunge wrote:

Found a typo in the PKGBUILD, which makes it impossible to install this as is.

conflics=("glibc")

should be

conflicts=("glibc")

Thanks for spotting these. I'll have to test back locally.

Offline

#12 2021-11-15 15:41:54

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

One more correction:

provides=("glibc")

needs to be

provides=("glibc=2.33")

otherwise pacman will refuse to replace glibc due to dependency resolution issues when things depends on for example glibc>2.0

I've now got it to build and install.

Offline

#13 2021-11-15 19:11:18

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

And I can confirm that it works, I've got DRM working again on kodi!

Offline

#14 2021-11-15 20:38:28

Xyne
Moderator/TU
Registered: 2008-08-03
Posts: 6,707
Website

Re: Request - PKGBUILD Review

The "arch" array only lists arm7h but the pkgbuild checks for arm, arm6h, arm7h and aarch64 in the build function. Which architectures are actually supported?
Is this an ARM-exclusive package? If so, the discussion should probably be moved to the Arch Linux ARM forums.

As for AUR support, ARM-only packages are effectively tolerated but any conflict with x86_64 packages, including ones in the official repos, will be resolved in favor of the x86_64 packages. That is to say, if someone wants to submit a glibc-widevine package for x86_64, this one may be removed if it doesn't also support x86_64.


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#15 2021-11-15 22:44:27

fedev
Member
Registered: 2017-09-21
Posts: 12

Re: Request - PKGBUILD Review

Xyne wrote:

The "arch" array only lists arm7h but the pkgbuild checks for arm, arm6h, arm7h and aarch64 in the build function. Which architectures are actually supported?
Is this an ARM-exclusive package? If so, the discussion should probably be moved to the Arch Linux ARM forums.

As for AUR support, ARM-only packages are effectively tolerated but any conflict with x86_64 packages, including ones in the official repos, will be resolved in favor of the x86_64 packages. That is to say, if someone wants to submit a glibc-widevine package for x86_64, this one may be removed if it doesn't also support x86_64.

This is an ARM only package. The ArchWiki said that AUR packages should be reviewed here, hence the post.

As for the package, it is mostly verbatim copy of the official package (except for the patches). I can definitely incorporate any valid feedback to it.

Offline

#16 2021-11-28 19:21:23

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

I had to rebuild and I can't get the original recipe working, I get a compile error:

In file included from dynamic-link.h:99,
                 from dl-load.c:60:
get-dynamic-info.h: In function ‘elf_get_dynamic_info’:
get-dynamic-info.h:107:24: error: ‘DT_RELR’ undeclared (first use in this function); did you mean ‘DT_RELA’?
  107 |       ADJUST_DYN_INFO (DT_RELR);
      |                        ^~~~~~~

I used this files as a starting point: https://github.com/archlinuxarm/PKGBUIL … core/glibc (commit cbfe362ef2dfa05e4028984725a4f41b4467e2d4).
Then applied the changes mentioned in the starting post, adapted the compile flags as described above, and updated the md5sums.

I also tried the patch from https://sourceware.org/pipermail/libc-a … 31768.html,  but there were too many changes which couldn't be applied.

UPDATE: I overlooked the post of ErwinJunge, I'm trying again with his patch change.

Last edited by pejobo (2021-11-29 10:57:23)

Offline

#17 2021-11-29 10:50:51

ErwinJunge
Member
Registered: 2018-02-01
Posts: 9

Re: Request - PKGBUILD Review

pejobo wrote:

I had to rebuild and I can't get the original recipe working, I get a compile error:

In file included from dynamic-link.h:99,
                 from dl-load.c:60:
get-dynamic-info.h: In function ‘elf_get_dynamic_info’:
get-dynamic-info.h:107:24: error: ‘DT_RELR’ undeclared (first use in this function); did you mean ‘DT_RELA’?
  107 |       ADJUST_DYN_INFO (DT_RELR);
      |                        ^~~~~~~

I used this files as a starting point: https://github.com/archlinuxarm/PKGBUIL … core/glibc (commit cbfe362ef2dfa05e4028984725a4f41b4467e2d4).
Then applied the changes mentioned in the starting post, adapted the compile flags as described above, and updated the md5sums.

I also tried the patch from https://sourceware.org/pipermail/libc-a … 31768.html,  but there were too many changes which couldn't be applied.

This happened to me as well. The answer is in https://bbs.archlinux.org/viewtopic.php … 6#p2003606

Offline

#18 2021-11-29 15:15:40

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

Thank you, I overlooked this post somehow. I could build and install it now (and wow, that took really long).
Have to test the function later and will update the post here.
When it works I will set up a github repository..

UPDATE:
DRM-Streams working again. Github link will follow later (hopefully today).

Last edited by pejobo (2021-12-01 14:56:27)

Offline

#19 2021-12-01 09:23:12

Xyne
Moderator/TU
Registered: 2008-08-03
Posts: 6,707
Website

Re: Request - PKGBUILD Review

fedev wrote:
Xyne wrote:

The "arch" array only lists arm7h but the pkgbuild checks for arm, arm6h, arm7h and aarch64 in the build function. Which architectures are actually supported?
Is this an ARM-exclusive package? If so, the discussion should probably be moved to the Arch Linux ARM forums.

As for AUR support, ARM-only packages are effectively tolerated but any conflict with x86_64 packages, including ones in the official repos, will be resolved in favor of the x86_64 packages. That is to say, if someone wants to submit a glibc-widevine package for x86_64, this one may be removed if it doesn't also support x86_64.

This is an ARM only package. The ArchWiki said that AUR packages should be reviewed here, hence the post.

As for the package, it is mostly verbatim copy of the official package (except for the patches). I can definitely incorporate any valid feedback to it.

That does't answer my question. There are several ARM architectures (e.g. arm6h, arm7h, aarch64). The PKGBUILD has code to check for all three of those and thus seems to support them, but the "arch" array only lists arm7h, which means that makepkg will only produce packages for arm7h. If the package only supports arm7h then the checks for arm6h and aarch64 should be removed. If it supports those then they should be added to the "arch" array.


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#20 2021-12-01 15:12:51

fedev
Member
Registered: 2017-09-21
Posts: 12

Re: Request - PKGBUILD Review

Xyne wrote:

That does't answer my question. There are several ARM architectures (e.g. arm6h, arm7h, aarch64). The PKGBUILD has code to check for all three of those and thus seems to support them, but the "arch" array only lists arm7h, which means that makepkg will only produce packages for arm7h. If the package only supports arm7h then the checks for arm6h and aarch64 should be removed. If it supports those then they should be added to the "arch" array.

What I meant was that I didn't change the original pkgbuild file that much and what you were pointing out was also not done in the original file (which is in the official ARM repo).

The original pkgbuild has "arch=(x86_64)" and then does all the arm checks.

Is the original pkgbuild file from the repos wrong or should I set x86_64 instead of arm7h?

Offline

#21 2021-12-01 16:22:47

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

I collected everything discussed here in this repository: https://github.com/pejobo/glibc-widevine
I will give permissions to this generously, just create a ticket on the repro.

For AUR I think a correct value for "arch" is preferred. I set PKGBUILD to "armv6h armv7h aarch64". But I cannot test for "armv6h" and "aarch64", we need feedback from others on this. Then maybe we can even set it to "any"..

Last edited by pejobo (2021-12-01 16:26:09)

Offline

#22 2021-12-01 16:26:29

Xyne
Moderator/TU
Registered: 2008-08-03
Posts: 6,707
Website

Re: Request - PKGBUILD Review

fedev wrote:

Is the original pkgbuild file from the repos wrong or should I set x86_64 instead of arm7h?

Setting arch to x86_64 should prevent makepkg from building an ARM package. I see that the original official PKGBUILD for Arch Linux ARM uses x86_64. At first I thought it may be a mistake but the other official packages use the same setting. It seems that array doesn't work the same way for their PKGBUILDs, or they populate the array automatically later in the build process.

Our official policy is to include the architectures for which the package can be build: https://wiki.archlinux.org/title/PKGBUILD#arch

Even if Arch Linux ARM ignores those values, it's useful to have the correct metadata available for the package in the AUR.


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#23 2021-12-01 17:42:35

pejobo
Member
Registered: 2015-04-25
Posts: 7

Re: Request - PKGBUILD Review

Xyne wrote:

It seems that array doesn't work the same way for their PKGBUILDs, or they populate the array automatically later in the build process.

I think the latter. The packages there are all pre-build with some magic.

Xyne wrote:

Even if Arch Linux ARM ignores those values, it's useful to have the correct metadata available for the package in the AUR.

It's definitly necessary because AUR packages on arm work the same as on X86. I build quite a lot of AUR packages in the past after changing the arch field locally..

Offline

Board footer

Powered by FluxBB