Rename PNaCl

Rename PNaCl from "linux_arm-untrusted" to:

pnacl_linux_i686   (on x86-32)
pnacl_linux_x86_64 (on x86-64)
pnacl_darwin_i386  (on Mac)
Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=4826

[jasonwkim, 2011-04-12 00:15:00]

On 2011/04/12 00:10:21, pdox wrote:

How does this interact with the (future) OStype naming change?
-jason

[pdox, 2011-04-12 00:16:42]

Jason,
Should be unrelated.

[jasonwkim, 2011-04-12 00:19:07]

http://codereview.chromium.org/6825060/diff/1/tools/toolchain_tester/toolchai...
File tools/toolchain_tester/toolchain_config.py (right):

http://codereview.chromium.org/6825060/diff/1/tools/toolchain_tester/toolchai...
tools/toolchain_tester/toolchain_config.py:171: DRIVER_PATH =
'toolchain/pnacl_linux_x86_64/bin'
Can we change these to use a single variable that is interpolated?

http://codereview.chromium.org/6825060/diff/2001/tools/toolchain_tester/toolc...
tools/toolchain_tester/toolchain_config.py:177: PNACL_LIB_DIR =
'toolchain/pnacl_linux_x86_64/libs-bitcode/'
Can we turn this in to a single var that is interpolated in?

[jasonwkim, 2011-04-12 00:19:46]

http://codereview.chromium.org/6825060/diff/2001/tests/pnacl_ld_example/Makef...
File tests/pnacl_ld_example/Makefile.pnacl (right):

http://codereview.chromium.org/6825060/diff/2001/tests/pnacl_ld_example/Makef...
tests/pnacl_ld_example/Makefile.pnacl:10:
PNACL_LD=../../toolchain/pnacl_linux_x86_64/bin/pnacl-ld
Here too

[pdox, 2011-04-12 00:23:59]

Jason,

Because those are already broken (in that they use a hard-coded path) and not
part of the bot testing framework, I'd prefer to postpone fixing those until
another CL. This one is already large and complex enough...

[jasonwkim, 2011-04-12 00:37:18]

On Mon, Apr 11, 2011 at 5:23 PM,  <pdox@google.com> wrote:
> Jason,
>
> Because those are already broken (in that they use a hard-coded path) and
> not
> part of the bot testing framework, I'd prefer to postpone fixing those until
> another CL. This one is already large and complex enough...

How big do you think the path hardcoding fix will be?

>
> http://codereview.chromium.org/6825060/
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[jasonwkim, 2011-04-12 00:38:13]

On Mon, Apr 11, 2011 at 5:16 PM,  <pdox@google.com> wrote:
> Jason,
> Should be unrelated.

If that's the case. then there isn't/shouldn't be any
conflation/leakage/hardcoding of the triple's OSType name to any of
the build/test scripts outside of binutils and llvm-gcc.... Right?

-jason

>
> http://codereview.chromium.org/6825060/
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[pdox, 2011-04-12 00:40:31]

Jason,
This is completely unrelated. I'm changing a directory name. I am not changing
any of the configure or make parameters of LLVM.

[bradn, 2011-04-12 00:47:41]

http://codereview.chromium.org/6825060/diff/2003/build/download_toolchains.py
File build/download_toolchains.py (right):

http://codereview.chromium.org/6825060/diff/2003/build/download_toolchains.py...
build/download_toolchains.py:24: 'x86-32':
['linux_x86','pnacl_linux_i686','linux_arm-trusted'],
why not drop the arm one?

http://codereview.chromium.org/6825060/diff/2003/build/download_toolchains.py...
build/download_toolchains.py:54: if 'arm' in flavor or 'pnacl' in flavor:
Why not drop all the arm stuff?

http://codereview.chromium.org/6825060/diff/2003/tests/pnacl_client_translato...
File tests/pnacl_client_translator/nacl.scons (right):

http://codereview.chromium.org/6825060/diff/2003/tests/pnacl_client_translato...
tests/pnacl_client_translator/nacl.scons:60: toolchain_dir = env['PNACL_ROOT']
Why not '${PNACL_ROOT} '?

[pdox, 2011-04-12 00:53:38]

http://codereview.chromium.org/6825060/diff/2003/build/download_toolchains.py
File build/download_toolchains.py (right):

http://codereview.chromium.org/6825060/diff/2003/build/download_toolchains.py...
build/download_toolchains.py:54: if 'arm' in flavor or 'pnacl' in flavor:
Because of linux_arm-trusted ?

http://codereview.chromium.org/6825060/diff/2003/tests/pnacl_client_translato...
File tests/pnacl_client_translator/nacl.scons (right):

http://codereview.chromium.org/6825060/diff/2003/tests/pnacl_client_translato...
tests/pnacl_client_translator/nacl.scons:60: toolchain_dir = env['PNACL_ROOT']
On 2011/04/12 00:47:41, bradn wrote:
> Why not '${PNACL_ROOT} '?

Done.

[jasonwkim, 2011-04-12 00:56:51]

On Mon, Apr 11, 2011 at 5:40 PM,  <pdox@google.com> wrote:
> Jason,
> This is completely unrelated. I'm changing a directory name. I am not
> changing
> any of the configure or make parameters of LLVM.

I am trying to understand the scope of this CL --
What I am thinking of is that I see similar changes needed when the
OSType change does occur to binutils and llvm-gcc
For example:

the untrusted arm tools are currently installed in arm-none-linux-gnueabi

This CL is changing the name of the outer directory, is that right?
(and only for linux_arm-untrusted) -

Does it touch the name of the downloaded toolchain directory?

>
> http://codereview.chromium.org/6825060/
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[bradn, 2011-04-12 00:56:51]

LGTM

[jvoung - send to chromium..., 2011-04-12 01:30:37]

http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
File tests/pnacl_client_translator/nacl.scons (right):

http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
tests/pnacl_client_translator/nacl.scons:53: # TODO(jvoung): rename the
"arm-untrusted" part to "pnacl-untrusted"
Can you remove this TODO?

http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common
File tests/spec2k/Makefile.common (right):

http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common...
tests/spec2k/Makefile.common:26:
NNACL_ROOT=$(TC_ROOT)/$(SCONS_BUILD_PLATFORM)_x86
Why not make BUILD_PLATFORM and SCONS_BUILD_PLATFORM the same (only "darwin" vs
"mac"?)

[jvoung - send to chromium..., 2011-04-12 01:34:45]

Also, kind of wish that the subarch was more canonical instead of i386 sometimes
and i686 sometimes.

On 2011/04/12 01:30:37, jvoung wrote:
>
http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
> File tests/pnacl_client_translator/nacl.scons (right):
> 
>
http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
> tests/pnacl_client_translator/nacl.scons:53: # TODO(jvoung): rename the
> "arm-untrusted" part to "pnacl-untrusted"
> Can you remove this TODO?
> 
> http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common
> File tests/spec2k/Makefile.common (right):
> 
>
http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common...
> tests/spec2k/Makefile.common:26:
> NNACL_ROOT=$(TC_ROOT)/$(SCONS_BUILD_PLATFORM)_x86
> Why not make BUILD_PLATFORM and SCONS_BUILD_PLATFORM the same (only "darwin"
vs
> "mac"?)

[pdox, 2011-04-12 01:41:10]

http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
File tests/pnacl_client_translator/nacl.scons (right):

http://codereview.chromium.org/6825060/diff/4018/tests/pnacl_client_translato...
tests/pnacl_client_translator/nacl.scons:53: # TODO(jvoung): rename the
"arm-untrusted" part to "pnacl-untrusted"
On 2011/04/12 01:30:38, jvoung wrote:
> Can you remove this TODO?

Done.

http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common
File tests/spec2k/Makefile.common (right):

http://codereview.chromium.org/6825060/diff/4018/tests/spec2k/Makefile.common...
tests/spec2k/Makefile.common:26:
NNACL_ROOT=$(TC_ROOT)/$(SCONS_BUILD_PLATFORM)_x86
Because "darwin" is the correct thing to use in a triple, not "mac".

"mac" is a marketing brand dating back to 1984. I'm not sure why we treat this
so sloppily to begin with.

[pdox, 2011-04-12 01:45:30]

Jan,

There's a precise and well-understood difference between i386 and i686.

Again, I'm not sure why scons creates its own imprecise architecture categories
instead of using the one's that already exist. Hopefully one day that'll be
fixed.