Page MenuHomeFreeBSD

net/py-libnet: Fix build for the platforms that ${MACHINE} != ${ARCH}
AbandonedPublic

Authored by loader on Jun 12 2018, 7:08 AM.

Details

Reviewers
koobs
sbz
Summary

${MACHINE} == ${ARCH} on i386 and amd64

distutils uses os.uname() machine in the build_lib path
https://github.com/python/cpython/blob/2.7/Lib/distutils/util.py#L65
https://github.com/python/cpython/blob/2.7/Lib/distutils/command/build.py#L82

Proposed commit log message:

net/py-libnet: Fix build for the platforms that ${MACHINE} != ${ARCH}

Reviewed_by: koobs, sbz
Approved by: koobs (mentor), sbz (maintainner)
Differential_Revision: D15771
Test Plan
  • portlint: OK (

WARN: /usr/ports/net/py-libnet/files/patch-src__builders.c: patch was not generated using `make makepatch''. It is recommended to use `make makepatch'' when you need to [re-]generate a patch to ensure proper patch format.
0 fatal errors and 1 warning found.
)

  • testport: OK (poudriere: 1200065, [armv7, aarch64, mips64], '', tested)

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17191
Build 17041: arc lint + arc unit

Event Timeline

loader created this revision.Jun 12 2018, 7:08 AM
loader edited the summary of this revision. (Show Details)Jun 12 2018, 7:12 AM
loader edited the summary of this revision. (Show Details)Jun 12 2018, 7:16 AM
loader edited the summary of this revision. (Show Details)Jun 12 2018, 7:29 AM
koobs requested changes to this revision.Jun 23 2018, 10:55 AM

Please include root cause / explanations in the commit log message as well.

Am I understanding correctly and completely that:

  1. MACHINE is equal to ARCH is only true on i386/amd64, AND
  2. MACHINE is not equal to ARCH for other (!i386/amd64) architectures, AND
  3. MACHINE is the correct variable to pass to python for ALL architectures?

Also, it seems I've had a WIP on this port, upgrading it to 3.0rc* and removing the custom do-install in favour of distutils autoplist concurrent.

I wonder if doing that bypasses the need to manually encode the directory names in the port entirely, or whether there's still an issue with !i386/amd64 even in that case.

This revision now requires changes to proceed.Jun 23 2018, 10:55 AM
loader abandoned this revision.Jun 23 2018, 4:26 PM

Please include root cause / explanations in the commit log message as well.
Am I understanding correctly and completely that:

  1. MACHINE is equal to ARCH is only true on i386/amd64, AND
  2. MACHINE is not equal to ARCH for other (!i386/amd64) architectures, AND

Sorry, that was my fault. To be accurate, besides i386/i386 and amd64/amd64
there are still a few platforms that also have MACHINE equal to ARCH like
mips/mips, powerpc/powerpc or sparc64/sparc64.

The platforms listed in the TEST PLAIN above are arm/armv7, arm64/aarch64 and mips/mips64.

% egrep 'define[[:space:]](MACHINE|MACHINE_ARCH)[[:space:]]' /usr/src/sys/*/include/param.h
/usr/src/sys/amd64/include/param.h:#define      MACHINE         "amd64"
/usr/src/sys/amd64/include/param.h:#define      MACHINE_ARCH    "amd64"
/usr/src/sys/arm/include/param.h:#define        MACHINE         "arm"
/usr/src/sys/arm/include/param.h:#define        MACHINE_ARCH    "arm" _V_SUFFIX _EB_SUFFIX
/usr/src/sys/arm64/include/param.h:#define      MACHINE         "arm64"
/usr/src/sys/arm64/include/param.h:#define      MACHINE_ARCH    "aarch64"
/usr/src/sys/i386/include/param.h:#define MACHINE               "i386"
/usr/src/sys/i386/include/param.h:#define       MACHINE_ARCH    "i386"
/usr/src/sys/mips/include/param.h:# define MACHINE      "mips"
/usr/src/sys/mips/include/param.h:# define MACHINE_ARCH         "mips" _N64_SUFFIX _EL_SUFFIX _HF_SUFFIX
/usr/src/sys/powerpc/include/param.h:#define    MACHINE         "powerpc"
/usr/src/sys/powerpc/include/param.h:#define    MACHINE_ARCH    "powerpc64"
/usr/src/sys/powerpc/include/param.h:#define    MACHINE_ARCH    "powerpcspe"
/usr/src/sys/powerpc/include/param.h:#define    MACHINE_ARCH    "powerpc"
/usr/src/sys/riscv/include/param.h:#define      MACHINE         "riscv"
/usr/src/sys/riscv/include/param.h:#define      MACHINE_ARCH    "riscv64"
/usr/src/sys/sparc64/include/param.h:#define MACHINE            "sparc64"
/usr/src/sys/sparc64/include/param.h:#define    MACHINE_ARCH    "sparc64"
  1. MACHINE is the correct variable to pass to python for ALL architectures?

Yes, because Python distutils also use it in the build_lib path.

Makefile variable ${MACHINE} is equal to uname(1) -m which gets value of utsname.machine through uname(3).
Makefile variable ${MACHINE_ARCH} and ${ARCH} are botch equal to uname(1) -p which gets the value of hw.machine_arch through sysctl(3).

${MACHINE} and ${MACHINE_ARCH} have a default value during compile time
https://svnweb.freebsd.org/base/head/contrib/bmake/main.c?revision=325340&view=markup#l1052
https://svnweb.freebsd.org/base/head/contrib/bmake/main.c?revision=325340&view=markup#l1085

${ARCH} is defined in Mk/bsd.ports.mk: ARCH!= ${UNAME} -p
https://svnweb.freebsd.org/ports/head/Mk/bsd.port.mk?revision=472890&view=markup#l1141

Python distutils uses the value of machine from os.uname() in the build_lib path
https://github.com/python/cpython/blob/2.7/Lib/distutils/util.py#L65
https://github.com/python/cpython/blob/2.7/Lib/distutils/command/build.py#L82

and os.uname() also fetches the value of utsname.machine through uname(3).
https://github.com/python/cpython/blob/2.7/Modules/posixmodule.c#L2894

For example, on arm64/aarch64, setup.py writes the output to
${WRKSRC}/build/lib.freebsd-12.0-CURRENT-arm64-2.7/libnet.so

creating build/lib.freebsd-12.0-CURRENT-arm64-2.7                                              
cc -shared -O2 -pipe -fno-strict-aliasing build/temp.freebsd-12.0-CURRENT-arm64-2.7/src/libnetm
odule.o -L/usr/local/lib -L/usr/local/lib -lnet -lpython2.7 -o build/lib.freebsd-12.0-CURRENT-a
rm64-2.7/libnet.so

but the do-install target tries to copy it from here:
${WRKSRC}/build/lib.freebsd-12.0-CURRENT-aarch64-2.7/libnet.so

Also, it seems I've had a WIP on this port, upgrading it to 3.0rc* and removing the custom do-install in favour of distutils autoplist concurrent.
I wonder if doing that bypasses the need to manually encode the directory names in the port entirely, or whether there's still an issue with !i386/amd64 even in that case.

Yes, USE_PYTHON=autoplist should fix this. Looks like you already have a plan on upgrading this port, I think I can just close the review now. :)

koobs added a comment.Jul 11 2018, 5:40 AM

I can arc create the current version of the diff if you'd like?

I can arc create the current version of the diff if you'd like?

Feel free to mail me a patch if you need someone test it on armv7/aarch64 :-)