[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH]libtorrent: FTBFS on hurd-i386: error: 'IPV6_TCLASS' was not
From: |
Jessica Clarke |
Subject: |
Re: [PATCH]libtorrent: FTBFS on hurd-i386: error: 'IPV6_TCLASS' was not declared in this scope. |
Date: |
Thu, 29 Apr 2021 15:55:57 +0100 |
On 29 Apr 2021, at 12:48, haha wang <hahwang@yandex.com> wrote:
>
> Package: libtorrent
> Severity: important
> Version: 0.13.8-2
> Tags: patch
> User:hahwang@yandex.com
> Usertags: hurd
> X-Debbugs-CC: debian-hurd@lists.debian.org
>
>
> I decide to fix a broken package found at the recommended page
> https://people.debian.org/~sthibault/out_of_date2.txt named `libtorrent`.
>
> After I download the package source and try to build without any
> modifications under the debian hurd running in qemu
> (debian-hurd-20210219.img), I got the following error.
>
> ```
> socket_fd.cc: In member function 'bool
> torrent::SocketFd::set_priority(torrent::SocketFd::priority_type)':
> socket_fd.cc:78:43: error: 'IPV6_TCLASS' was not declared in this scope;
> did you mean 'IPOPT_CLASS'?
> ```
>
> and it matches with what that page said.
>
> I also try to build that package under my debian desktop(Debian GNU/Linux
> bullseye/sid x86_64) and it got no errors. After a quick search, i have found
> linux defined the `IPV6_TCLASS` macro at `bits/in.h` as follows:
>
> ```
> #define IPV6_TCLASS 67
> ```
>
> So that, I modify the `configure.ac` to add a conditional compilation when it
> detects the host operating system is hurd based, along with macro definition
> at the `Makefile.am` found at `src/net/`. The patch file is attached at the
> end of this email.
>
> Hope for reviews!
>
> Thank you!
>
> ---
> hahawang
>
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,17 @@
> AC_INIT(libtorrent, 0.13.8, sundell.software@gmail.com)
>
> +AC_CANONICAL_HOST
> +
> +build_hurd=no
> +
> +case "${host_os}" in
> + gnu*)
> + build_hurd=yes
> + ;;
> +esac
> +
> +AM_CONDITIONAL([BUILD_HURD], [test "$build_hurd" = "yes"])
> +
> LT_INIT([disable-static])
>
> dnl Find a better way to do this
> --- a/src/net/Makefile.am
> +++ b/src/net/Makefile.am
> @@ -26,3 +26,7 @@
> throttle_node.h
>
> AM_CPPFLAGS = -I$(srcdir) -I$(srcdir)/.. -I$(top_srcdir)
> +
> +if BUILD_HURD
> +AM_CPPFLAGS += -DBUILD_HURD
> +endif
> --- a/src/net/socket_fd.cc
> +++ b/src/net/socket_fd.cc
> @@ -50,6 +50,10 @@
> #include "torrent/exceptions.h"
> #include "socket_fd.h"
>
> +#ifdef BUILD_HURD
> +#define IPV6_TCLASS 67
> +#endif
> +
> namespace torrent {
>
> inline void
This is completely wrong.
1. Configure scripts should almost never check the OS, instead it should check
for specific things (“does X exist?”, "does Y work?” etc). Checking for an OS
is not general, breaking on not-yet seen OSes, and also brittle as OSes can
change what they support. If you instead check for the feature itself then
it will always work everywhere.
2. IPV6_TCLASS is an optional feature supported by some OSes, and has an
OS-specific value (e.g. it’s 67 on Linux but 61 on FreeBSD). If Hurd doesn’t
provide it, then using a random Linux-specific value for it is complete
nonsense and at best will just give an error, but at worst will silently end
up doing something completely unknown if the value 67 happens to mean
something else.
The *correct* thing to do is to check whether IPV6_TCLASS exists in the
configure script and if it doesn’t disable use of it in socket_fd.cc.
Also, your message was an HTML email, which tends to garble patches, causes
inconsistent text formatting, is a pain for people who use plain-text email
clients and, most importantly, does not work for the Debian Bug Tracker, as it
needs to parse the Package: etc headers in the body and that just doesn’t work
when it’s arbitrary HTML instead of well-structured plain text.
Jess