Bug 238353 - devel/gn: respect CXX and CXXFLAGS
Summary: devel/gn: respect CXX and CXXFLAGS
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Jan Beich
URL:
Keywords: patch, patch-ready
Depends on:
Blocks:
 
Reported: 2019-06-05 22:19 UTC by Jan Beich
Modified: 2019-06-19 11:56 UTC (History)
3 users (show)

See Also:
o.hushchenkov: maintainer-feedback+


Attachments
v1 (has commit message) (3.20 KB, patch)
2019-06-05 22:19 UTC, Jan Beich
o.hushchenkov: maintainer-approval+
Details | Diff
poudriere log with "make test" enabled (163.77 KB, text/plain)
2019-06-05 22:21 UTC, Jan Beich
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2019-06-05 22:19:32 UTC
Created attachment 204848 [details]
v1 (has commit message)

When a poudriere jail is built with -x (native-xtools) some variables CC/CXX/etc are pointing to the cross-compilers under /nxb-bin in order to reduce qemu-user-static overhead. Besides, users may want to switch compiler, pass non-default flags (e.g., for debugging) or Clang may not be available on some architectures.

http://pylon.nyi.freebsd.org/data/head-powerpc64-default/p503023_s348376/logs/errors/gn-1529.log
https://github.com/DragonFlyBSD/DeltaPorts/commit/710854931a4aefe809ab8e2d0c9cd6fb29a4beae

GCC failure was:
FAILED: util/exe_path.o
g++9 -MMD -MF util/exe_path.o.d  -I.. -I. -O2 -pipe -fstack-protector-strong -Wl,-rpath=/usr/local/lib/gcc9 -DNDEBUG -O3 -fdata-sections -ffunction-sections -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -pthread -pipe -fno-exceptions -fno-rtti -fdiagnostics-color -O2 -pipe -fstack-protector-strong -Wl,-rpath=/usr/local/lib/gcc9 -Wl,-rpath=/usr/local/lib/gcc9 -std=c++14 -Wno-c++11-narrowing -c ../util/exe_path.cc -o util/exe_path.o
../util/exe_path.cc: In function 'base::FilePath GetExePath()':
../util/exe_path.cc:56:12: error: 'PATH_MAX' was not declared in this scope
   56 |   char buf[PATH_MAX];
      |            ^~~~~~~~
../util/exe_path.cc:58:22: error: 'buf' was not declared in this scope
   58 |   if (sysctl(mib, 4, buf, &buf_size, nullptr, 0) == -1) {
      |                      ^~~
../util/exe_path.cc:61:25: error: 'buf' was not declared in this scope
   61 |   return base::FilePath(buf);
      |                         ^~~
At global scope:
cc1plus: warning: unrecognized command line option '-Wno-c++11-narrowing'
Comment 1 Jan Beich freebsd_committer 2019-06-05 22:21:07 UTC
Created attachment 204849 [details]
poudriere log with "make test" enabled

diff --git a/devel/gn/Makefile b/devel/gn/Makefile
index f5ce4432ad7e..350cadb951e3 100644
--- a/devel/gn/Makefile
+++ b/devel/gn/Makefile
@@ -33,4 +33,8 @@ do-install:
 do-test:
 	${TEST_WRKSRC}/gn_unittests
 
+pre-install:	do-test
+
 .include <bsd.port.mk>
+
+do-test:	.IGNORE
Comment 2 Jan Beich freebsd_committer 2019-06-05 22:29:08 UTC
www/firefox may depend on devel/gn in future, see
https://bugzilla.mozilla.org/show_bug.cgi?id=1534615

Piotr, can you check this is enough for powerpc64? Also try "make test".
Comment 3 Carlos J. Puga Medina freebsd_committer 2019-06-06 07:38:52 UTC
There is a typo in CONFLICTS_INSTALL

s/chromimum-gn/chromium-gn
Comment 4 Oleh Hushchenkov 2019-06-06 10:48:35 UTC
I am fine with the changes.

As already noted cpm@ - there is my typo in "chromiMum-gn".

I think a part of these can be upstreamed.

Thank you for fixes!
Comment 5 Oleh Hushchenkov 2019-06-06 10:53:14 UTC
(In reply to Jan Beich from comment #1)
After we switched to python3 as default some tests fail. With python2 all tests passed.
Comment 6 commit-hook freebsd_committer 2019-06-09 04:31:23 UTC
A commit references this bug:

Author: jbeich
Date: Sun Jun  9 04:30:47 UTC 2019
New revision: 503780
URL: https://svnweb.freebsd.org/changeset/ports/503780

Log:
  devel/gn: modernize build

  - Unbreak with GCC
  - Unbreak on DragonFly
  - Allow Python 3.x
  - Allow debug builds
  - Convert to HAS_CONFIGURE
  - Convert to do-build from USES=ninja
  - Drop default build dependency in do-test
  - Drop unnecessary glob in CONFLICTS_INSTALL

  PR:		238353
  Reviewed by:	cpm
  Approved by:	Oleh Hushchenkov (maintainer)

Changes:
  head/devel/gn/Makefile
  head/devel/gn/files/patch-build_gen.py
  head/devel/gn/files/patch-tools_gn_exec__process__unittest.cc
  head/devel/gn/files/patch-util_exe__path.cc
Comment 7 Jan Beich freebsd_committer 2019-06-09 04:34:00 UTC
(In reply to Oleh Hushchenkov from comment #4)
> I think a part of these can be upstreamed.

I can't. Upstreaming to Google projects requires Google Account (to sign CLA, interact with Gerrit) which is impossible to sign up for via Tor.
https://lists.torproject.org/pipermail/tor-talk/2012-October/025923.html

(In reply to Oleh Hushchenkov from comment #5)
> After we switched to python3 as default some tests fail.

Fixed. All tests now pass in poudriere on 12.0 aarch64/armv7.

Testing on 11.* exposed an (unrelated) issue:

[245/512] NinjaBuildWriterTest.GetSelfInvocationCommandLine
*** FAILURE ../tools/gn/ninja_build_writer_unittest.cc:25: file_.IsValid()
*** FAILURE ../tools/gn/ninja_build_writer_unittest.cc:66: "../../testdot.gn" == cmd_out.GetSwitchValueASCII(switches::kDotfile)
Comment 8 Oleh Hushchenkov 2019-06-09 11:45:43 UTC
(In reply to Jan Beich from comment #7)
> I can't. Upstreaming to Google projects requires Google Account (to sign CLA, interact with Gerrit) which is impossible to sign up for via Tor.

I will try to send appropriate patches to upstream.
Comment 9 Jan Beich freebsd_committer 2019-06-13 19:23:41 UTC
I've successfully built devel/gn on ref12-ppc64.freebsd.org by copying WRKSRC after "USE_GCC=yes make configure". However, runtime is partially broken. What Linux implies by "uname -m" usually means "uname -p" on FreeBSD. As such host architecture is only properly detected on i386, amd64, arm* (any 32bit), aarch64 and mips (o32). Even then aarch64 is detected as kArm aka 32bit arm.

  $ uname -mp
  powerpc powerpc64

  $ ./gn_unittests
  [20/512] Analyze[0x81160a000:0613/185628.894605:FATAL:args.cc(357)] Check failed: false. OS architecture not handled. (powerpc)
  [22/512] ArgsTest.VerifyOverrideScope
  Abort trap (core dumped)

  $ UNAME_m=ppc64 ./gn_unittests
  [512/512] XmlElementWriter.TestXmlEscape
  PASSED

  # List architectures where uname -m and uname -p show different values
  $ make targets -C/usr/src | awk -F/ '$1 !~ $2'
  Supported TARGET/TARGET_ARCH pairs for world and kernel targets
      arm/armv6
      arm/armv7
      arm64/aarch64
      mips/mipsel
      mips/mips64el
      mips/mips64
      mips/mipsn32
      mips/mipselhf
      mips/mipshf
      mips/mips64elhf
      mips/mips64hf
      powerpc/powerpc64
      powerpc/powerpcspe
      riscv/riscv64
Comment 10 Oleh Hushchenkov 2019-06-19 11:56:13 UTC
(In reply to Jan Beich from comment #9)
gn relies on uname(3) to determine the machine architecture. We can patch it like this:

diff --git a/util/sys_info.cc b/util/sys_info.cc
index a1ce3e92..92ae69d8 100644
--- a/util/sys_info.cc
+++ b/util/sys_info.cc
@@ -7,6 +7,11 @@
 #include "base/logging.h"
 #include "util/build_config.h"
 
+#if defined(OS_FREEBSD)
+#include <sys/sysctl.h>
+#include <sys/types.h>
+#endif
+
 #if defined(OS_POSIX)
 #include <sys/utsname.h>
 #include <unistd.h>
@@ -17,7 +22,24 @@
 #endif
 
 std::string OperatingSystemArchitecture() {
-#if defined(OS_POSIX)
+#if defined(OS_FREEBSD)
+  int mib[] = { CTL_HW, HW_MACHINE_ARCH };
+  char march[1024];
+  size_t march_size = 1024;
+  if (sysctl(mib, 2, march, &march_size, nullptr, 0) == -1) {
+    NOTREACHED();
+    return std::string();
+  }
+  std::string arch(march);
+  if (arch == "i386") {
+    arch = "x86";
+  } else if (arch == "amd64") {
+    arch = "x86_64";
+  } else if (arch == "powerpc64") {
+    arch = "ppc64";
+  }
+  return arch;
+#elif defined(OS_POSIX)
   struct utsname info;
   if (uname(&info) < 0) {
     NOTREACHED();

But I have only amd64 machines and can't test it. Maybe we need to create different PR for this issue.