Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GN]: GN fails to build on AIX #16

Open
jbajwa opened this issue Nov 10, 2016 · 16 comments
Open

[GN]: GN fails to build on AIX #16

jbajwa opened this issue Nov 10, 2016 · 16 comments
Assignees
Labels

Comments

@jbajwa
Copy link

jbajwa commented Nov 10, 2016

I've updated the configs for GN to build on PPC/s390 linux (CL), but on AIX it requires more effort as one of its dependencies (libevent) is failing to build.

Script to build GN standalone using ninja:

#!/bin/bash

set -e
set -v

# Build Ninja and update PATH
git clone https://github.com/martine/ninja.git -b v1.7.1
cd ninja && ./configure.py --bootstrap
cd ..
export PATH="$PWD/ninja:$PATH"

# Get the sources
mkdir gn-standalone
cd gn-standalone
mkdir tools
cd tools
git clone https://chromium.googlesource.com/chromium/src/tools/gn
cd ..
mkdir -p third_party/libevent
cd third_party/libevent
wget --no-check-certificate https://chromium.googlesource.com/chromium/chromium/+archive/master/third_party/libevent.tar.gz
tar -xvzf libevent.tar.gz
cd ../..
git clone https://chromium.googlesource.com/chromium/src/base
git clone https://chromium.googlesource.com/chromium/src/build
git clone https://chromium.googlesource.com/chromium/src/build/config
mkdir testing
cd testing
git clone https://chromium.googlesource.com/chromium/testing/gtest
cd ..

# Build
cd tools/gn
./bootstrap/bootstrap.py -s

# At this point, the resulting binary is at:
# gn-standalone/out/Release/gn

info about ninja: https://ninja-build.org/manual.html
info about gn: https://chromium.googlesource.com/chromium/src/tools/gn

@liKelvinKh
Copy link

Currently failing on "wget --no-check-certificate https://chromium.googlesource.com/chromium/chromium/+archive/master/third_party/libevent.tar.gz"
Reason: wget in dal-pax.canlab and dal-pacha.canlab do not have HTTPS (SSL/TLS) options, and so do not have the --no-check-certificate flag. reinstalling wget might resolve the problem

@jbajwa
Copy link
Author

jbajwa commented Nov 11, 2016

@liKelvinKh For now can you download the tarball on another machine and copy over?

@liKelvinKh
Copy link

okay, I will do that

@jbajwa
Copy link
Author

jbajwa commented Nov 11, 2016

Thanks, basically to get the environment set in-order to run bootstrap script, you can update the script as follows (assuming you copy the tarball in ~/yourlocaldir/):

mkdir -p third_party/libevent
cd third_party/libevent
cp ~/yourlocaldir/libevent.tar.gz .
tar -xvzf libevent.tar.gz
cd ../..

@jbajwa
Copy link
Author

jbajwa commented Nov 14, 2016

What error do you get with the debugger? It is possible that the bootstrap script is missing logic for AIX.
Also the error you get is simply because you don't have the build.ninja file which ninja requires, the bootstrap script should generate that first and then call ninja.
I will touch base with you in person.

@liKelvinKh
Copy link

Log: it complained that the system cannot find 'cc'. I added logic so that when it is AIX, it uses gcc instead of cc.

@liKelvinKh
Copy link

liKelvinKh commented Nov 17, 2016

Findings:

/home/lkelvinkh/gn-standalone/base/third_party/libevent/select.c:67:23: error: conflicting types for 'fd_mask'
typedef unsigned long fd_mask;
In file included from /home/lkelvinkh/gn-standalone/base/third_party/libevent/select.c:40:0:
/usr/include/sys/select.h:47:14: note: previous declaration of 'fd_mask' was here
typedef long fd_mask;

edit: Disregard the following, there isn't a select.h in libevent
select.c includes select.h, but I think it is using the wrong select.h file as there is a select.h inside libevent directory.

@joransiu
Copy link
Member

@liKelvinKh: does it allow you to override with the CC / CXX environment variables?

@liKelvinKh
Copy link

liKelvinKh commented Nov 17, 2016

@joransiu : It didn't override it, but it added a default value in case the environment variable was not found.
I added the following:

if is_aix:
cc = os.environ.get('CC', 'gcc')
else:
cc = os.environ.get('CC', 'cc')

os.environ.get('CC') looks for an environment variable 'CC'. And if it is not found, it uses the second parameter instead as a default value.

@liKelvinKh
Copy link

/home/lkelvinkh/gn-standalone/base/third_party/libevent/evrpc.c:133:28: error: 'next' undeclared (first use in this function)
TAILQ_FOREACH(hook, head, next) {

Getting this error. Ran the original command and nothing new shows up. It appears that "next" exists in a struct defined in evrpc.h:

struct evrpc {
TAILQ_ENTRY(evrpc) next;

or

struct evrpc_request_wrapper {
TAILQ_ENTRY(evrpc_request_wrapper) next;

Here is one of the pieces of code in evrpc that uses "next":
static int

evrpc_remove_hook_internal(struct evrpc_hook_list *head, void *handle)
{
struct evrpc_hook *hook = NULL;
TAILQ_FOREACH(hook, head, next) {
if (hook == handle) {
TAILQ_REMOVE(head, hook, next);
free(hook);
return (1);
}
}
return (0);
}

@jbajwa
Copy link
Author

jbajwa commented Nov 21, 2016

@liKelvinKh
Check the libevent aix/config.h (the new one you created), if "TAILQ_FOREACH" is not defined in sys/queue.h then maybe you to undef it there.

@liKelvinKh
Copy link

@jbajwa but variable "next" is still undeclared though?

@jbajwa
Copy link
Author

jbajwa commented Nov 22, 2016

Discussed with @liKelvinKh , the next step would be to get the correct config.h file for libevent. The one generated by the bundled libevent is not complete. Will have to build libevent on AIX from https://github.com/libevent/libevent and copy over the config files.

jbajwa pushed a commit that referenced this issue Dec 29, 2016
…iew.chromium.org/2536463002/ )

Reason for revert:
Breaks layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/11861

See:
https://github.com/v8/v8/wiki/Blink-layout-tests

Original issue's description:
> Object
> -- New JSObject for promises: JSPromise
>
> Builtins
> -- PromiseThen TFJ
> -- PromiseCreateAndSet TFJ for internal use
> -- PerformPromiseThen TFJ for internal use
> -- PromiseInit for initial promise setup
> -- SpeciesConstructor for use in PromiseThen
> -- ThrowIfNotJSReceiver for use in SpeciesConstructor
> -- AppendPromiseCallback to update FixedArray with new callback
> -- InternalPerformPromiseThen
>
> Promises.js
> -- Cleanup unused symbols
> -- Remove PerformPromiseThen
> -- Remove PromiseThen
> -- Remove PromiseSet
> -- Remove PromiseAttachCallbacks
>
> Runtime
> -- PromiseSet to set promise inobject values
> -- Refactor functions to use FixedArrays for callbacks instead of
>    JSArray
> -- Runtime_PromiseStatus to return promise status
> -- Runtime_PromiseResult to return promise result
> -- Runtime_PromiseDeferred to return deferred attached to promise
> -- Runtime_PromiseRejectReactions to return reject reactions attached
>    to promise
>
> This CL results in a 13.07% improvement in the promises benchmark
> (over 5 runs).
>
> BUG=v8:5343
>
> Committed: https://crrev.com/30b564c76f490f8f6b311a74b25b26cf0a96be2d
> Cr-Commit-Position: refs/heads/master@{#41503}

[email protected],[email protected],[email protected],[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5343

Review-Url: https://codereview.chromium.org/2554013002
Cr-Commit-Position: refs/heads/master@{#41512}
jbajwa pushed a commit that referenced this issue Dec 29, 2016
….chromium.org/2536463002/ )"

This reverts commit 4c7cccf.

BUG=v8:5343

Review-Url: https://codereview.chromium.org/2554943002
Cr-Commit-Position: refs/heads/master@{#41534}
jbajwa pushed a commit that referenced this issue Jan 20, 2017
…ps://codereview.chromium.org/2549773002/ )

Reason for revert:
gc stress failures:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20gc%20stress/builds/8024

Original issue's description:
> Internalize strings in-place
>
> using newly introduced ThinStrings, which store a pointer to the actual,
> internalized string they represent.
>
> BUG=v8:4520
>
> Review-Url: https://codereview.chromium.org/2549773002
> Cr-Commit-Position: refs/heads/master@{#42168}
> Committed: https://chromium.googlesource.com/v8/v8/+/af51befe694fe039db3554d4b9165f7d6baceb77

[email protected],[email protected],[email protected],[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4520

Review-Url: https://codereview.chromium.org/2621913002
Cr-Commit-Position: refs/heads/master@{#42170}
@rbanerjee
Copy link

rbanerjee commented Mar 11, 2017

The required repositories from chromium to build GN -

https://github.com/martine/ninja.git
https://chromium.googlesource.com/chromium/src/tools/gn
https://chromium.googlesource.com/chromium/src/base
https://chromium.googlesource.com/chromium/src/build
https://chromium.googlesource.com/chromium/src/build/config
https://chromium.googlesource.com/chromium/testing/gtest

Below are initial patches with a link to where the master lives on googlesource.

base_patch.txt | https://chromium.googlesource.com/chromium/src/base/+/master | commit to apply on top of - cd3764c886341eea637a92960e92f8668d61ec42
build_config_patch.txt | https://chromium.googlesource.com/chromium/src/build/config/+/master | commit to apply on top of - 87a16ead26af86046bb265535ff770860978dfeb
build_patch.txt | https://chromium.googlesource.com/chromium/src/build/+/master | commit to apply on top of - 87a16ead26af86046bb265535ff770860978dfeb
tools_gn_patch.txt | https://chromium.googlesource.com/chromium/src/tools/gn/+/master" | commit to apply on top of - 10547d2db2e74817aec32dd47f8d2f6b98b2069a

These changes make it possible to build gn on aix::ppc64. The generated GN works on my initial testcases including an attempt to build v8 with it. Currently we don't get any sort of errors or warnings while building v8. Of course changes had to be made to v8 (mostly config files) to achieve that. We can go through those patches after we are done contributing these. One step at a time I guess. : ]

These are still somewhat rough and probably can be polished a lot further. Any sort of feedback would be great.

@rbanerjee
Copy link

rbanerjee commented Mar 15, 2017

Patches for building v8 using the aforementioned GN ( the one built on aix ) -

v8_patch.txt | https://chromium.googlesource.com/v8/v8.git/+/master | commit to apply on top of - c9e83ebc3947aca22d024129963e1e7d25d804acc

build_patch.txt | https://chromium.googlesource.com/chromium/src/+/master/build/ | commit to apply on top of - 0fdcf96e9928229dd4b3b366b00a49da8eae00ed

icu_patch.txt | commit to apply on top of - ec5152fccfdf72281af53f863e3859c20f409153

@rbanerjee
Copy link

rbanerjee commented May 30, 2017

This is now complete with the required patches committed to Google's repository.

The GN binary was ported for AIX along with linux_s390x, linux_ppc64 and linux_ppc64le. ( See: https://chromium.googlesource.com/chromium/src/+/0088ee5017f4bef6c27243a54d8998f993db11b8).

The patches required to build v8 with using aforementioned GN binary have also went though. Since they touch projects on both V8 and Chromium (for utilizing the already existing build toolchain), this had to be done in multiple cls which are listed below.
https://chromium.googlesource.com/chromium/src/+/367a04209fcb6c3700daee65511945da7dd31f20
https://chromium.googlesource.com/v8/v8/+/468f1958e01b80bacd61bff809bfae4312d45e3e
https://chromium.googlesource.com/chromium/deps/icu/+/c5f212bac791598c3367d9318c3452b47944cf54

Also, note that it's up to us to keep the GN binary building on our supported platforms (similar to how we currently deal with V8).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants