Fix ndless_installer_4.5 on macOS by unilock · Pull Request #200 · ndless-nspire/Ndless

unilock · 2020-02-20T01:22:18Z

Mostly untested. Assumes the following have been installed with brew:

binutils
boost
gcc
git
gmp
libmpc
mpfr
wget
zlib

May break if more than one gcc version is installed.

adriweb · 2020-02-20T12:19:19Z

I'll try that later as I need to update my toolchain anyway and I previously made some changes about that...
I'm not sure everything in the patch is actually needed. I built the toolchain just fine with clang, which allows to avoid all the gcc version stuff altogether since it'll take the default xcode one.
I believe the only change I previously needed to make was to remove with-python from the GDB build options. Which, funnily enough, isn't part of this patch.

Vogtinator · 2020-02-20T12:29:35Z

+OS_NAME := $(shell uname -s | tr A-Z a-z)
 payload: ndless_installer.bin
-	(../tools/MakeChunkDispatch/MakeChunkDispatch 0x1800e7f4 0x12480000; cat $^) | base64 -w0 > $@ || rm $@
+	ifeq ($(OS_NAME),darwin)


This should be done by using feature detection instead. base64 doesn't have/need the -w0 option on some Linux distros either.

Vogtinator · 2020-02-20T12:32:41Z

I don't think the brew specific changes make sense. AFAIK brew (optionally?) sets up $PATH, so it should already find everything correctly.

The unalias calls in ./bulid_toolchain.sh are noops anyway as they only influence the current shell, which is exited directly afterwards.

adriweb · 2020-02-20T22:37:18Z

So, I've been able to use build_toolchain.sh just fine on macOS today, I just updated the versions of the deps: #201
I also removed --with-python because I had problems in the past with it.

So I guess your modifications aren't actually needed? Must be some configuration on your side, for brew etc. I don't believe I have anything special in my PATH to make it work...

Regarding the base64 thing in installer-4.5, yeah, we need to have a solution properly, as it wouldn't work on macOS otherwise.

Anyway, I agree with what @Vogtinator said, otherwise.

unilock · 2020-02-21T02:42:02Z

@adriweb That's strange; I ran into errors when first attempting to build the toolchain with clang. Though, trying it again with only the changes from #201 applied, I'm able to build it just fine (notably with --with-python).
So my changes to build_toolchain.sh were unnecessary.

Vogtinator

Can you drop the changes to build_toolchain.sh and make the installer-4.5/Makefile use feature detection? Then I'll merge.

unilock · 2020-06-25T01:11:54Z

@Vogtinator I believe I've done as you asked; I'm not 100% certain if my modification actually works. Notably, while rebuilding the toolchain on macOS to test my handiwork, I ran into the following issue:

Extracting binutils-2.34.tar.bz2...
tar: Option -a is not permitted in mode -x

Removing -a from tar in build_toolchain.sh seems to have fixed this. Not sure why it was there in the first place.

adriweb · 2020-06-25T01:15:10Z

That's probably due to the other recent changes, uh oh :p

unilock · 2020-06-25T01:44:25Z

ifeq ($(shell base64 -w 2>/dev/null),) is probably more correct than ifeq ($(base64 -w 2>/dev/null),) - but apparently still incorrect, as I run into

ifeq (,)
/bin/sh: -c: line 0: syntax error near unexpected token `,'
/bin/sh: -c: line 0: `ifeq (,)'

when trying to compile my crummy code.
If someone who knows what they're doing wants to implement "feature detection" for the base64 command, by all means, go ahead.

I can at least say with confidence that the toolchain compiles successfully on macOS, once -a is removed from tar.

Vogtinator · 2020-06-25T06:24:39Z

ifeq ($(shell base64 -w 2>/dev/null),) is probably more correct than ifeq ($(base64 -w 2>/dev/null),)

Yeah, I don't think $(base64 ...) has the desired effect as there's no base64 make builitin.

but apparently still incorrect, as I run into

That means it was run as part of the rule command passed to sh, maybe just drop the indentation? See https://www.gnu.org/software/make/manual/html_node/Recipe-Syntax.html#Recipe-Syntax. If that doesn't work, you could do the conditional evaluation as part of the shell recipe (though everything has to be on a single logical line).

I can at least say with confidence that the toolchain compiles successfully on macOS, once -a is removed from tar.

For autodetection of the compression algorithm, but that might not . Just to be sure, you didn't change anything other than -xaf -> -xf, right?

fridtjof · 2020-08-04T14:26:15Z

I can at least say with confidence that the toolchain compiles successfully on macOS, once -a is removed from tar.

For autodetection of the compression algorithm, but that might not . Just to be sure, you didn't change anything other than -xaf -> -xf, right?

Apparently, the tar macOS uses (BSD tar) always auto detects compression formats without any special parameters:

https://www.freebsd.org/cgi/man.cgi?tar(1)

CTRL+F for "compression automatically when reading archives"

I can also confirm removing -a does the trick on macOS.

macOS failure is expected at this time, see ndless-nspire#200 (comment)

Seems like both BSD and GNU tar autodetect the method for decompression, but BSD tar doesn't like the -a option for extraction. Fixes part of #200

Vogtinator · 2020-08-05T11:21:37Z

I can also confirm removing -a does the trick on macOS.

Even better, so both BSD and GNU tar are happy with just -xf. I dropped the -a now.

macOS failure is expected at this time, see ndless-nspire#200 (comment)

Kakise · 2020-10-14T06:13:38Z

ifeq ($(shell base64 -w 2>/dev/null),) is probably more correct than ifeq ($(base64 -w 2>/dev/null),)

Yeah, I don't think $(base64 ...) has the desired effect as there's no base64 make builitin.

but apparently still incorrect, as I run into

That means it was run as part of the rule command passed to sh, maybe just drop the indentation? See https://www.gnu.org/software/make/manual/html_node/Recipe-Syntax.html#Recipe-Syntax. If that doesn't work, you could do the conditional evaluation as part of the shell recipe (though everything has to be on a single logical line).

I can at least say with confidence that the toolchain compiles successfully on macOS, once -a is removed from tar.

For autodetection of the compression algorithm, but that might not . Just to be sure, you didn't change anything other than -xaf -> -xf, right?

This:

BASE64 := base64 -w0
ifeq ($(shell base64 -w 2> /dev/null),)
    BASE64 := base64
endif

works perfectly on macOS

N0ury · 2020-10-14T06:18:47Z

ifeq ($(shell base64 -w 2>/dev/null),) is probably more correct than ifeq ($(base64 -w 2>/dev/null),)

Yeah, I don't think $(base64 ...) has the desired effect as there's no base64 make builitin.

but apparently still incorrect, as I run into

That means it was run as part of the rule command passed to sh, maybe just drop the indentation? See https://www.gnu.org/software/make/manual/html_node/Recipe-Syntax.html#Recipe-Syntax. If that doesn't work, you could do the conditional evaluation as part of the shell recipe (though everything has to be on a single logical line).

I can at least say with confidence that the toolchain compiles successfully on macOS, once -a is removed from tar.

For autodetection of the compression algorithm, but that might not . Just to be sure, you didn't change anything other than -xaf -> -xf, right?

This:
BASE64 := base64 -w0
ifeq ($(shell base64 -w 2> /dev/null),)
    BASE64 := base64
endif
works perfectly on macOS

This is probably not native base64.

base64 -w0
base64: invalid option -- w
Usage:	base64 [-hvDd] [-b num] [-i in_file] [-o out_file]
  -h, --help     display this message
  -Dd, --decode   decodes input
  -b, --break    break encoded string into num character lines
  -i, --input    input file (default: "-" for stdin)
  -o, --output   output file (default: "-" for stdout)

Kakise · 2020-10-14T06:39:58Z

This is probably not native base64.

base64 -w0
base64: invalid option -- w
Usage:	base64 [-hvDd] [-b num] [-i in_file] [-o out_file]
  -h, --help     display this message
  -Dd, --decode   decodes input
  -b, --break    break encoded string into num character lines
  -i, --input    input file (default: "-" for stdin)
  -o, --output   output file (default: "-" for stdout)

Tried with a fresh macOS 10.15.7 install.
Can you try this patch https://gist.github.com/Kakise/637740402370369366cd95e125e64ccb ? It should work

Vogtinator · 2020-10-17T18:58:57Z

Can you try this patch https://gist.github.com/Kakise/637740402370369366cd95e125e64ccb ? It should work

That would break build on systems where -w0 is necessary, here the condition is also true.

* Add a CI workflow for the toolchain and SDK * CI: fix toolchain permissions * CI: use tar to accelerate artifacts * CI: temporarily force packaging the toolchain, and gz it * CI: skip packaging again if there's a cache * CI: fix paths * CI: fix artifact download path * CI: minor formatting * CI: expand SDK build to more OSs macOS failure is expected at this time, see #200 (comment) * CI: remove macOS for now more steps (including a separate toolchain build for macOS) are needed for this * CI: install necessary dependencies * CI: separate toolchain for each OS * CI: add Ubuntu 20.04 * CI: use matrix.os instead of runner.os * Update .github/workflows/main.yml use -j4 for make Co-authored-by: Fabian Vogt <fabian@ritter-vogt.de> * CI: merge jobs into one * CI: always install toolchain dependencies * CI: clarify a step * CI: remove ubuntu-16.04 * CI: simplify artifact upload * Revert "CI: simplify artifact upload" This reverts commit 8c01113. * ci: remove artifact uploads * CI: update PATH setup to use environment files see https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

Kakise · 2020-10-19T19:12:39Z

Can you try this patch https://gist.github.com/Kakise/637740402370369366cd95e125e64ccb ? It should work

That would break build on systems where -w0 is necessary, here the condition is also true.

How would it break it ? The "BASE64" variable is defaulted to base64 -w0and is changed only if the -w option is not implemented. Or am I missing something ? Genuinely asking because I'm bad with makefiles lol

Vogtinator · 2020-10-19T19:33:28Z

Can you try this patch https://gist.github.com/Kakise/637740402370369366cd95e125e64ccb ? It should work

That would break build on systems where -w0 is necessary, here the condition is also true.

How would it break it ? The "BASE64" variable is defaulted to base64 -w0and is changed only if the -w option is not implemented. Or am I missing something ? Genuinely asking because I'm bad with makefiles lol

The detection doesn't work. Here the w option is implemented and ifeq ($(shell base64 -w 2> /dev/null),) is still true.

…se64 command Signed-off-by: unilock <unilock@example.com>

unilock · 2020-10-20T21:46:30Z

The latest hastily-pushed commit should work. I didn't have time to test in the context of building Ndless.

Vogtinator · 2020-10-24T15:25:49Z

The latest hastily-pushed commit should work. I didn't have time to test in the context of building Ndless.

It doesn't seem to work. base64 -w exits with an error here:

> base64 -w
base64: option requires an argument -- 'w'
Try 'base64 --help' for more information.
> echo $?
1

unilock · 2020-10-24T16:14:49Z

It doesn't seem to work. base64 -w exits with an error here:

Ah, that's because -w expects a valid wrap size. Try the new commit.
Only tested on Linux; not macOS. But it "should" work on both.

Vogtinator · 2020-10-25T10:07:34Z

Only tested on Linux; not macOS. But it "should" work on both.

Yeah, confirmed to work here. The main point of this is to fix it for macOS, so this really should be tested there before merging.

unilock · 2020-10-25T14:35:54Z

Seems like it's working on my end.

Vogtinator · 2020-10-25T14:52:41Z

Ok.

The use of $? with pipes can be a bit weird, so could you optimize the pipe away, like base64 -w0 >/dev/null 2>&1 </dev/null?

`-w` expects a valid wrap size

unilock · 2020-10-25T15:24:15Z

Done. I can confirm that works on both Linux and macOS.

Vogtinator reviewed Feb 20, 2020

View reviewed changes

Vogtinator requested changes Jun 14, 2020

View reviewed changes

unilock changed the title ~~Fix building toolchain + ndless_installer_4.5 on macOS~~ Fix ndless_installer_4.5 on macOS Jun 25, 2020

Vogtinator mentioned this pull request Jun 29, 2020

SDK Error: "Make" Command on Top-Level Directory of Repository Fails #88

Closed

fridtjof added a commit to shinyblink/Ndless that referenced this pull request Aug 4, 2020

CI: expand SDK build to more OSs

89fed50

macOS failure is expected at this time, see ndless-nspire#200 (comment)

Vogtinator added a commit that referenced this pull request Aug 5, 2020

build_toolchain.sh: Call tar -xf without -a

ee7dbb1

Seems like both BSD and GNU tar autodetect the method for decompression, but BSD tar doesn't like the -a option for extraction. Fixes part of #200

fridtjof added a commit to shinyblink/Ndless that referenced this pull request Oct 6, 2020

CI: expand SDK build to more OSs

ade4707

macOS failure is expected at this time, see ndless-nspire#200 (comment)

Vogtinator mentioned this pull request Oct 17, 2020

[macOS] bug in the Makefile for the Ndless 4.5 installer #249

Closed

unilock closed this Oct 20, 2020

unilock reopened this Oct 20, 2020

ndless/src/installer-4.5/Makefile: implement feature detection for ba…

342845d

…se64 command Signed-off-by: unilock <unilock@example.com>

ndless/src/installer-4.5/Makefile: fix feature detection for base64

5e0fae8

`-w` expects a valid wrap size

Vogtinator approved these changes Oct 25, 2020

View reviewed changes

Vogtinator merged commit 8d9206c into ndless-nspire:master Oct 25, 2020

Conversation

unilock commented Feb 20, 2020

Uh oh!

adriweb commented Feb 20, 2020

Uh oh!

Vogtinator Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

Vogtinator commented Feb 20, 2020

Uh oh!

adriweb commented Feb 20, 2020

Uh oh!

unilock commented Feb 21, 2020

Uh oh!

Vogtinator left a comment

Choose a reason for hiding this comment

Uh oh!

unilock commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriweb commented Jun 25, 2020 via email

Uh oh!

unilock commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vogtinator commented Jun 25, 2020

Uh oh!

fridtjof commented Aug 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vogtinator commented Aug 5, 2020

Uh oh!

Kakise commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

N0ury commented Oct 14, 2020

Uh oh!

Kakise commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vogtinator commented Oct 17, 2020

Uh oh!

Kakise commented Oct 19, 2020

Uh oh!

Vogtinator commented Oct 19, 2020

Uh oh!

unilock commented Oct 20, 2020

Uh oh!

Vogtinator commented Oct 24, 2020

Uh oh!

unilock commented Oct 24, 2020

Uh oh!

Vogtinator commented Oct 25, 2020

Uh oh!

unilock commented Oct 25, 2020

Uh oh!

Vogtinator commented Oct 25, 2020

Uh oh!

unilock commented Oct 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

unilock commented Jun 25, 2020 •

edited

Loading

unilock commented Jun 25, 2020 •

edited

Loading

fridtjof commented Aug 4, 2020 •

edited

Loading

Kakise commented Oct 14, 2020 •

edited

Loading

Kakise commented Oct 14, 2020 •

edited

Loading