Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 27 Sep 2023 02:10:31 +0000
From: Brian Cain <bcain@...cinc.com>
To: Rob Landley <rob@...dley.net>,
        "musl@...ts.openwall.com"
	<musl@...ts.openwall.com>,
        "Matheus Bernardino (QUIC)"
	<quic_mathbern@...cinc.com>
CC: Sid Manning <sidneym@...cinc.com>, Rich Felker <dalias@...c.org>,
        Fangrui
 Song <i@...kray.me>, Szabolcs Nagy <nsz@...t70.net>
Subject: RE: [RFC PATCH 0/5] Add support to Hexagon DSP



> -----Original Message-----
> From: Rob Landley <rob@...dley.net>
> Sent: Tuesday, September 26, 2023 8:49 PM
> To: Brian Cain <bcain@...cinc.com>; musl@...ts.openwall.com; Matheus
> Bernardino (QUIC) <quic_mathbern@...cinc.com>
> Cc: Sid Manning <sidneym@...cinc.com>; Rich Felker <dalias@...c.org>;
> Fangrui Song <i@...kray.me>; Szabolcs Nagy <nsz@...t70.net>
> Subject: Re: [musl] [RFC PATCH 0/5] Add support to Hexagon DSP
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
> 
> On 9/26/23 13:13, Brian Cain wrote:
> >> ./build-toolchain.sh: line 250: TOOLCHAIN_INSTALL: unbound variable
> >> $ TOOLCHAIN_INSTALL="$PWD"/walrus ./build-toolchain.sh
> >> ++ date +%Y_%b_%d
> >> + STAMP=2023_Sep_26
> >> + set -euo pipefail
> >> + set -x
> >> + set +x
> >> ./build-toolchain.sh: line 256: ROOT_INSTALL: unbound variable
> >>
> >
> > Ok - sure, let me take a minute to describe the usage of these scripts.
> Standby.
> 
> Sigh. It's better, but it's not exactly "./configure; make; make install"
> either. You're not making it easy for me to wrap your thing in an automated
> invocation script without installing Docker.
> 
> +## Usage
> +
> +Checkout the required source repos like `llvm-project`, `musl`, etc. Invoke
> 
> Does not say what they are, still does not provide sample invocation script that
> would work from a git clone without manual steps to be determined by the
> user.
> 
> +`get-src-tarballs.sh` with the corresponding `*_SRC_URL` links to the specific
> +releases to use (see `Dockerfile` for reference / last-known-good versions).
> 
> Corresponding. Hmmm...
> 
> $ grep SRC_URL Dockerfile
> ENV LLVM_SRC_URL https://github.com/llvm/llvm-project/archive/llvmorg-
> ${VER}.tar.gz
> ARG QEMU_SRC_URL=https://download.qemu.org/qemu-8.1.0.tar.xz
> ENV MUSL_SRC_URL
> https://github.com/quic/musl/archive/7243e0d3a9d7e0f08d21fc194a05749e0
> bb26725.tar.gz
> ENV LINUX_SRC_URL https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-
> 6.4.13.tar.xz
> #ENV PYTHON_SRC_URL https://www.python.org/ftp/python/3.9.5/Python-
> 3.9.5.tar.xz
> ENV BUSYBOX_SRC_URL https://busybox.net/downloads/busybox-
> 1.33.1.tar.bz2
> 
> One of those has an = and all the others have a space. The LLVM url has $VER in
> it where the others are explicit version numbers in the URL.
> 
> I was hoping for something like "env $(sed -n 's/.* \([^ ]*_SRC_URL\)[=
> ]\(.*\)/\1=\2/p' Dockerfile) ./get-src-tarballs.sh" without having to special
> case specific variables, but no. (And of course the one with the = is not the
> one with the $VER.)
> 
> $ grep -o '{.*SRC_URL}' get-src-tarballs.sh | sort -u
> {LINUX_SRC_URL}
> {LLVM_SRC_URL}
> {MUSL_SRC_URL}
> {QEMU_SRC_URL}
> 
> The get-src-tarballs.sh script isn't just downloading and extracting package
> source, it's also creating package.txt files in $MANIFEST_DIR, which is the
> first of 2 expected arguments. The script does not check if those arguments are
> provided, nor does it have help/usage info beyond "read the full source to the
> end".
> 
> Are these manifest files important, what does get-src-repos.sh... it calls wait
> 8 times, when "help wait" says "If ID is not given, waits for all currently
> active child processes, and the return status is zero" so 7 of those calls
> should be NOPs? Anyway, dump_checkout_info() is doing "git remote -v" to the
> same manifest text file names, so maybe that file is consumed by the later
> script. Although given that "remote -v info" output isn't obviously machine
> parseable, it looks like the consumer would  be catting it into a readme or
> something at best, so I could just echo "hello" to each of them if I just wanted
> to make it work quickly...
> 
> Sigh, any attempt to make a ./build.sh wrapper for this thing is going to be as
> brittle as my last build script, isn't it? I'll have to re-read all this stuff
> every git pull to see if it broke my wrapper...
> 
> +Or instead you can check out the trunk of those projects' repos using
> +`git` - try invoking `get_src_repos.sh`.
> 
> Which is nice to know, but "build random git snapshot du jour of everything in
> a
> previously never tried by any human combination and hope for the best" is
> seldom
> my first choice Let's see...
> 
> $ bash -c "$(sed -En 's/.* (VER|[^ ]*_SRC_URL)[= ](.*)/\1=\2/p' Dockerfile |
> xargs) ./get-src-tarballs.sh $PWD/src $PWD/manifest"
> + get_src_tarballs
> + cd /home/landley/toybox/toolchain_for_hexagon/src
> ./get-src-tarballs.sh: line 9: cd:
> /home/landley/toybox/toolchain_for_hexagon/src: No such file or directory
> 
> Really? It doesn't make the directories? Sigh...
> 
> bash -c "mkdir -p src manifest && $(sed -En 's/.* (VER|[^ ]*_SRC_URL)[=
> ](.*)/\1=\2/p' Dockerfile | xargs) ./get-src-tarballs.sh $PWD/src
> $PWD/manifest"
> 
> Your wget has --quiet on it. You _suppressed_ the progress indicator.
> 
> > * `ARTIFACT_TAG` - the tag from the llvm-project repo with which this release
> > should be labeled.
> 
> Does this have to be a tag in the repo, or is it just an arbitrary string? It
> looks like it's just used to set RESULTS_DIR_ which has a trailing underscore
> for reasons I do not understand...
> 
> RESULTS_DIR_=${ARTIFACT_BASE}/${ARTIFACT_TAG}
> mkdir -p ${RESULTS_DIR_}
> RESULTS_DIR=$(readlink -f ${RESULTS_DIR_})
> 
> Because you didn't want to RESULTS_DIR=$(readlink -f $RESULTS_DIR)
> 
> You've never tested any of these scripts on paths with spaces in them, have
> you?
> 
> > * `TOOLCHAIN_INSTALL` - the path to install the toolchain to.
> > * `ROOT_INSTALL` - the path to install the rootfs to.  Initially this will
> > only contain the target includes + libraries.
> > * `ARTIFACT_BASE` - the path to put the tarballs + manifests.
> > * optional `MAKE_TARBALLS` - if `MAKE_TARBALLS` is set to `1`, it will create
> > tarballs of the release and purge the intermediate build artifacts.
> >
> > Sample usage:
> >
> >     export ARTIFACT_TAG=17.0.0
> >     export TOOLCHAIN_INSTALL=$PWD/clang+llvm-${ARTIFACT_TAG}-cross-
> hexagon-unknown-linux-musl
> >     export ROOT_INSTALL=$PWD/install_rootfs
> >     export ARTIFACT_BASE=$PWD/artifacts
> >
> >     mkdir -p ${ARTIFACT_BASE}
> >
> >     ./build-toolchain.sh 2>&1 | tee build_${ARTIFACT_TAG}.log
> 
> Yay sample usage. Once again the script expects the directory to exist. I
> personally find prefix assignment useful in these sort of things (lifetime rules
> are kind of a big deal to me: where does data come from, how long does it last,
> when is it updated and by who, do the provider and consumer have an obvious
> relationship), but once again the script expects the directory to exist which
> makes prefix assignment more awkward here...
> 
> Hmmm, ROOT_INSTALL is used to set ROOTFS and ROOT_INSTALL_REL, neither
> of which
> are used again by the script and not exported either, so I THINK that's just
> debris in build-toolchain.sh?
> 
> ./build-toolchain.sh: line 256: ROOT_INSTALL: unbound variable
> 
> Except, of course, it fails if the unused variable is not set. Let's feed it
> ROOT_INSTALL=no and...
> 
> ./build-toolchain.sh: line 276: ccache: command not found
> 
> $ grep ccache build-toolchain.sh
> ccache --show-stats
> ccache --show-stats
> $ sed -i /ccache/d build-toolchain.sh
> $ ARTIFACT_TAG=17.0.0
> TOOLCHAIN_INSTALL=$PWD/clang+llvm-${ARTIFACT_TAG}-cross-hexagon-
> unknown-linux-musl
> ARTIFACT_BASE=$PWD/artifacts ROOT_INSTALL=no ./build-toolchain.sh 2>&1 |
> tee out.txt
> ++ date +%Y_%b_%d
> + STAMP=2023_Sep_26
> + set -euo pipefail
> + set -x
> + set +x
> $
> 
> And stick some "echo" in there... It silently exited after running "which
> clang", which is not installed on the host. The interesting part is this one
> DIDN'T complain about command not found, just silently died. I wonder why?
> 
> *shrug* I'm making progress, but I think I need to debootstrap a newer root
> filesystem version than the one I'm using before going much further, since you
> then call "python3.8" as a command name and this has 3.7.3 and can't apt-get
> anything newer without a major version update. (I'm still on devuan B and D
> just
> dropped, I've skipped the C release entirely. Busy with other things. Sigh, I
> should bite the bullet...)

Tsk...sorry, clearly there's lots of room for improvement in the build script.

> Still no qemu-system-hexagon I see. When did I last poke Taylor Simpson about
> that... 2021 it looks like:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg05062.html

Yeah, it's sadly not there yet.  We're making (glacial?) progress towards that goal.

> Thanks for the help. I'll let you know if I get it working...

I had hoped that binary builds of the toolchain might satisfy most of the interested parties.  But I suppose we've all read "Reflections on Trusting Trust" and understand the importance of being able to build it yourself.  So we'll take your feedback into account and try to make improvements.

-Brian

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.