mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Brian Cain <bcain@quicinc.com>
To: Rob Landley <rob@landley.net>,
	"musl@lists.openwall.com" <musl@lists.openwall.com>,
	"Matheus Bernardino (QUIC)" <quic_mathbern@quicinc.com>
Cc: Sid Manning <sidneym@quicinc.com>, Rich Felker <dalias@libc.org>,
	Fangrui Song <i@maskray.me>, Szabolcs Nagy <nsz@port70.net>
Subject: RE: [musl] [RFC PATCH 0/5] Add support to Hexagon DSP
Date: Wed, 27 Sep 2023 02:10:31 +0000	[thread overview]
Message-ID: <SN6PR02MB420548DE0863CE3677C8764BB8C2A@SN6PR02MB4205.namprd02.prod.outlook.com> (raw)
In-Reply-To: <03a1f7cd-12d4-9d53-4997-ebb820bd23c3@landley.net>



> -----Original Message-----
> From: Rob Landley <rob@landley.net>
> Sent: Tuesday, September 26, 2023 8:49 PM
> To: Brian Cain <bcain@quicinc.com>; musl@lists.openwall.com; Matheus
> Bernardino (QUIC) <quic_mathbern@quicinc.com>
> Cc: Sid Manning <sidneym@quicinc.com>; Rich Felker <dalias@libc.org>;
> Fangrui Song <i@maskray.me>; Szabolcs Nagy <nsz@port70.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

  reply	other threads:[~2023-09-27  2:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-30 12:22 Matheus Tavares Bernardino
2023-08-30 12:22 ` [musl] [RFC PATCH 1/5] Add support to Hexagon arch Matheus Tavares Bernardino
2023-08-30 12:22 ` [musl] [RFC PATCH 2/5] hexagon: add fenv header and implementation Matheus Tavares Bernardino
2023-08-30 12:22 ` [musl] [RFC PATCH 3/5] hexagon: add fma/fmaxf/fminf routines Matheus Tavares Bernardino
2023-08-30 12:22 ` [musl] [RFC PATCH 4/5] hexagon: add bits/user.h Matheus Tavares Bernardino
2023-08-30 12:22 ` [musl] [RFC PATCH 5/5] INSTALL: add 'Hexagon' to list of supported targets Matheus Tavares Bernardino
2023-09-08 11:18 ` [musl] [RFC PATCH 0/5] Add support to Hexagon DSP Matheus Tavares Bernardino
2023-09-26 16:43 ` Rob Landley
2023-09-26 16:48   ` Brian Cain
2023-09-26 17:55     ` Rob Landley
2023-09-26 18:13       ` Brian Cain
2023-09-27  0:05         ` Brian Cain
2023-09-27  1:49         ` Rob Landley
2023-09-27  2:10           ` Brian Cain [this message]
2023-09-27 13:19             ` Rob Landley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN6PR02MB420548DE0863CE3677C8764BB8C2A@SN6PR02MB4205.namprd02.prod.outlook.com \
    --to=bcain@quicinc.com \
    --cc=dalias@libc.org \
    --cc=i@maskray.me \
    --cc=musl@lists.openwall.com \
    --cc=nsz@port70.net \
    --cc=quic_mathbern@quicinc.com \
    --cc=rob@landley.net \
    --cc=sidneym@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).