@djames1 Please don't ping random people. Your PR will be reviewed when time is available.
That being said, this looks like its okay to merge; even if its doing scary things to the linker.
The one change I want is the license to come from somewhere that isn't the void-packages tree. Presumably this is in the source some where; please use that copy.
@the-maldridge the source tarball does not contain the license at all, however their github repository does. That is why I included the license file from their git repository. There are multiple files in the tree that include a license file in the ${FILESDIR}, examples being: discord, graphviz, btsync, ksh, shadow, hfsprogs, evilwm, lsof, mksh, and jenkins.
Attempting to lecture a maintainer on package policy tends to decrease the number of people willing to review your PRs...
If the license is at a fixed location you may use xbps-uhelper fetch to retrieve it in a post_build() from a URL based on the git tag just before using vlicense to install it. This is the preferred way to do it as if you version it in xbps-src and the license changes, it is very likely that the package will no longer carry with it the correct license terms.
As far as the packages above, we have several packages with -git versions as well and yet policy precludes git packages entirely. Not everything is perfectly in compliance at all times.
@the-maldridge I wasn't trying to come across as lecturing you. I was just wanting to make 100% sure that I was not being misguided in any way by reading other packages for guidance. I apologize if my comment sounded harsh, that was not my intention at all.
The git tag for release 0.0.1 does not include a license file either. When they release 0.0.2, I will change the package to pull the license from the tarball, as their changes since 0.0.1 add a LICENSE file.
Thanks!
Duly noted. As soon as https://github.com/AdelieLinux/gcompat/issues/2 closes please make sure the license is removed from our source tree and the correct path is used.
As this makes changes to the way the linker works I am going to request one other maintainer from @voidlinux/pkg-committers replies with a shipit before merging.
You can just add the license url to distfiles. distfiles and checksum are space separated lists. It requires a tiny bit more magic to make work, but it's all documented.
Vaelatern commented on this pull request.
> @@ -0,0 +1,32 @@
+# Template file for 'gcompat'
+pkgname=gcompat
+version=0.0.1
+revision=1
+wrksrc="${pkgname}-${pkgname}-${version}"
+only_for_archs="aarch64-musl armv6l-musl armv7l-musl i686-musl x86_64-musl"
and when we add a new musl arch, this will break, but it can stand for now.
the-maldridge commented on this pull request.
> @@ -0,0 +1,32 @@
+# Template file for 'gcompat'
+pkgname=gcompat
+version=0.0.1
+revision=1
+wrksrc="${pkgname}-${pkgname}-${version}"
+only_for_archs="aarch64-musl armv6l-musl armv7l-musl i686-musl x86_64-musl"
veto! this breaks my armv5tel repo. This should be done the correct way or at the least include mipsel and armvte{,l}.
Vaelatern commented on this pull request.
> +maintainer="Daniel James <djames@orcadian.net>"
+homepage="https://github.com/AdelieLinux/gcompat"
+license="ISC"
+distfiles="https://github.com/AdelieLinux/gcompat/archive/${pkgname}-${version}.tar.gz"
+checksum=138f4094977268a762415da10c05bcf87fae948faa8ff13a9b8b4ff4d64c91e6
+
+case "$XBPS_TARGET_MACHINE" in
+ aarch64-musl) _glibc="ld-linux-aarch64.so.1" _musl="ld-musl-aarch64.so.1";;
+ armv6l-musl) _glibc="ld-linux-armhf.so.3" _musl="ld-musl-armhf.so.1";;
+ armv7l-musl) _glibc="ld-linux-armhf.so.3" _musl="ld-musl-armhf.so.1";;
+ i686-musl) _glibc="ld-linux.so.2" _musl="ld-musl-x86.so.1";;
+ x86_64-musl) _glibc="ld-linux-x86-64.so.2" _musl="ld-musl-x86_64.so.1";;
+esac
+
+make_build_args="LINKER_PATH=/lib/${_musl} LOADER_NAME=${_glibc}"
+make_install_args="LINKER_PATH=/lib/${_musl} LOADER_NAME=${_glibc} PREFIX=/usr DESTDIR=${DESTDIR}"
`PREFIX` and `DESTDIR` definitely redundant.
As for the linker changes, I'll need to review further. Other maintainers: go right ahead and review yourself, I'm not going to review it today.
To give some insight onto what exactly happens here, this package creates a file called libgcompat.so.0 in /usr/lib. This file is a shim that basically translates some glibc functions into musl linker functions.
It then creates a symlink to that file with in the location that the glibc linker would be in if this were a glibc system. Since that file should not exist on a musl system, and all the binaries already on the musl system should be linked against musl's linker, there should be no linker issues.
My two cents: I believe that our musl port should be musl only. So even if we merge, there must be a rule that we only merge stuff that links directly to musl and must not use gcompat.
personally I would prefer to use a containerization framework like snap or flatpak for those apps instead.
I think that is a good idea, there should be a linter check that makes sure this isn't in any package's depends line.
While I like the idea of using containerization and chroots, I also have some software which aren't available in such formats and aren't good candidates for packaging due to commercial reasons.
Vaelatern commented on this pull request.
> @@ -0,0 +1,32 @@
+# Template file for 'gcompat'
+pkgname=gcompat
+version=0.0.1
+revision=1
+wrksrc="${pkgname}-${pkgname}-${version}"
+only_for_archs="aarch64-musl armv6l-musl armv7l-musl i686-musl x86_64-musl"
Then adding, for now, the `case` statement is the correct way to go, @djames1
Vaelatern commented on this pull request.
> +
+Redistributions in binary form must reproduce the above copyright
+notice, this list of conditions and the following disclaimers in
+the documentation and/or other materials provided with the distribution.=
+
+Neither the names of Ad=C3=A9lie Linux, nor the names of its contributor=
s, may
+be used to endorse or promote products derived from this Software withou=
t
+specific prior written permission.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+IN NO EVENT SHALL THE CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE
+FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+WITH THE SOFTWARE.
Can we please get this license from outside our repo?
-- =
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/voidlinux/void-packages/pull/7707#pullrequestreview-63=
986248=
@Vaelatern when I try to pull LICENSE file from https://raw.githubusercontent.com/AdelieLinux/gcompat/master/LICENSE I get the following error: `ERROR: gcompat-0.0.1_1: unknown distfile suffix for LICENSE.`
After looking at https://github.com/voidlinux/void-packages/blob/master/Manual.md I see that the file extension needs to be in the extension list there. Is there another way to do this?
@the-maldridge that seems to have worked. PR should be ready for review again.
Summary of changes:
- Added support for all musl archs that we currently support
- Modified template to fetch LICENSE from AdelieLinux's repo
- Changed do_install to post_install
- Added more make_build_args and make_install_args to fix errors in build I didn't catch originally.