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

Fails to build in path containing spaces when no system TCL is available #199

Closed
lurch opened this issue Apr 7, 2021 · 4 comments
Closed

Comments

@lurch
Copy link

lurch commented Apr 7, 2021

Disclaimer: I'm not a TCL programmer, and I've never used TCL directly, but it's causing occasional build-problems for https://github.com/raspberrypi/pico-setup (where it's a dependency of openocd).

If there's a space in the filepath containing the jimtcl folder, when you try to build you get an error like:

	MKLDEXT	_load-static-exts.c
/bin/sh: 1: /tmp/some: not found
Makefile:103: recipe for target '_load-static-exts.c' failed
make: *** [_load-static-exts.c] Error 1

because the full-path isn't being properly quoted in the Makefile.

When I tried reproducing this on my main Ubuntu laptop, I was surprised to discover that it built fine even when I had spaces in my filepath! But then when I looked at the Makefile I noticed that it was then using my system-installed tclsh (which is in a path without any spaces), and that probably explains why this bug has been undiscovered for so long! Only after I'd uninstalled all my "system tcl" programs, was I able to reproduce the problem on my Ubuntu laptop.

So for now, the easiest way to reproduce is using something like Docker.
Fire up a Debian docker image using sudo docker run -it debian and then within that docker image run the following sequence of commands:

apt update
apt install -y build-essential git
mkdir /tmp/some_path
cd /tmp/some_path
git clone https://github.com/msteveb/jimtcl.git
cd jimtcl
./configure
make

This shows that jimtcl is able to successfully build inside this Docker image in a path without spaces.

But if you then do:

mkdir '/tmp/some other path'
cd '/tmp/some other path'
git clone https://github.com/msteveb/jimtcl.git
cd jimtcl
./configure
make

it instantly fails with

	MKLDEXT	_load-static-exts.c
/bin/sh: 1: /tmp/some: not found
make: *** [Makefile:104: _load-static-exts.c] Error 1

I did have a quick go at fixing this myself (so that I could submit a PR), but I didn't have any idea what all the autosetup stuff was doing, which is why I'm submitting an issue instead 😉

@msteveb
Copy link
Owner

msteveb commented Apr 7, 2021

Thanks for the report. This could be fixed with the following change:

diff --git a/auto.def b/auto.def
index 3f2f5a0c..347951ff 100644
--- a/auto.def
+++ b/auto.def
@@ -296,7 +296,7 @@ switch -glob -- $host_os {

 # Find some tools
 cc-check-tools ar ranlib strip
-define tclsh [info nameofexecutable]
+define tclsh [quote-if-needed [info nameofexecutable]]

 # We only support silent-rules for GNU Make
 define NO_SILENT_RULES

However trying to build in a directory with spaces in the name is just asking for trouble. It will likely break all manner of things.

@lurch
Copy link
Author

lurch commented Apr 8, 2021

This could be fixed with the following change:

Thanks Steve! That fixes the build-problem, and I'd never have figured out how to do that myself 😄
However when I look in the Makefile that gets generated, I see that it still has

$(DEF_LD_PATH) $(MAKE) -C tests jimsh=/tmp/some other path/jimtcl/jimsh

and

CC='cc' /tmp/some other path/jimtcl/configure

which presumably also need to be quoted?

However trying to build in a directory with spaces in the name is just asking for trouble.

Normally I'd agree with you, however you can never predict what a user might try... raspberrypi/pico-setup#17

It will likely break all manner of things.

According to the extensive testing done by @michaelstoops , jimtcl is the only part of our setup process that breaks with spaces-in-the-path.

EDIT: And tests/Makefile also has

	@$(DEF_LD_PATH) $(jimsh) /tmp/some other path/jimtcl/tests/runall.tcl

and

	@rc=0; for i in /tmp/some other path/jimtcl/tests/*.test; do $(tclsh) -encoding utf-8 $$i || rc=$?; done; exit $$rc

@msteveb msteveb closed this as completed in bcd4434 Apr 8, 2021
@msteveb
Copy link
Owner

msteveb commented Apr 8, 2021

I've pushed some changes. Hopefully this will fix all the problems.

@lurch
Copy link
Author

lurch commented Apr 9, 2021

Fantastic, thanks so much @msteveb ! 👍

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

No branches or pull requests

2 participants