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

external/Makefile: Change MAKE_HOST and BUILD to CROSS_* #1533

Closed
wants to merge 1 commit into from
Closed

external/Makefile: Change MAKE_HOST and BUILD to CROSS_* #1533

wants to merge 1 commit into from

Conversation

jsarenik
Copy link
Contributor

MAKE_HOST is internally defined by GNU Make. See
http://git.savannah.gnu.org/cgit/make.git/commit/?id=ef11217

Checked on Alpine Linux locally.

I am sorry for not checking the presence of MAKE_HOST variable
in my first commit. Thanks to @NicolasDorier for pointing it out.

Ref: #1318 #1231

@jsarenik
Copy link
Contributor Author

@ZmnSCPxj see this

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jun 1, 2018

tACK, this fix my issue

@ZmnSCPxj
Copy link
Contributor

ZmnSCPxj commented Jun 1, 2018

I would prefer autotools, but I think @rustyrussell is intending to create a configure script for this.

This is OK to me if it fixes a problem in our Makefile. Cannot really check in detail as of now due to lack of access to my hacking computer.

utACK 708edeb

@jsarenik
Copy link
Contributor Author

jsarenik commented Jun 1, 2018

@ZmnSCPxj I see, but we are not there yet and all I wanted was to do a minimal fix that makes sure both FreeBSD and Musl systems work.

The problem lies actually in the Autoconf-enabled submodules which have a very outdated config.guess and config.sub. These are static files that need to be updated in-source of libbacktrace, but can be updated in-place like this:

cd ~/src/lightning
#git clean -xfd # this is how I clean locally, but following would be better:
git add -A
git stash
git submodule deinit --all -f
git checkout 27a186be # the state with MAKE_HOST
git submodule update --init --recursive
sh /tmp/config-fix.sh
make -j4

And here is the config-fix.sh script:

#!/bin/sh

MYLOC=/tmp
URL=https://git.savannah.gnu.org/gitweb/

myget() {
  wget -q --content-disposition \
    "$URL?p=config.git;a=blob_plain;f=config.$1" \
    && echo File config.$1 downloaded
}
cd /tmp; myget guess; myget sub; cd - >/dev/null 2>&1

find . -name 'config.guess' | while read target
do
  dir=${target%/*}
  cp -f $MYLOC/config.guess $MYLOC/config.sub $dir/
  echo Files in $dir updated
  cd - >/dev/null 2>&1
done
rm $MYLOC/config.sub $MYLOC/config.guess

@jsarenik
Copy link
Contributor Author

jsarenik commented Jun 1, 2018

@NicolasDorier ^^ above in-place change of config.{guess,sub} files is a way to compile on Musl system without the patch in this pull-request.

@jsarenik
Copy link
Contributor Author

jsarenik commented Jun 1, 2018

If the above pull-request to libbacktrace is accepted, the submodule should be updated and that update commited to our .gitmodules file. Then this patch is not needed and rather should not be merged.

Still, the fact that ./configure script in libbacktrace is always (by our external/Makefile) called with --host=$MAKE_HOST is rather non-standard I would say (in case of no cross-compilation). Luckily when the config.guess and config.sub scripts are up-to-date (and know musl) they can cope with it.

There is a way to add --host flag to ./configure only when needed, but in that case MAKE_HOST can not be used because this is always defined. It would be something like:

beginning of external/Makefile:

ifdef CROSS_HOST
HOST_ADD := --host=$(CROSS_HOST)
endif

...

... ./configure ... $(HOST_ADD) ...
...

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jun 2, 2018

Not very familiar with build systems in C++, so I wonder: What is the difference between updating libbacktrace to allow -musl and keep as now, versus not updating it and use CROSS_HOST.

@jsarenik
Copy link
Contributor Author

jsarenik commented Jun 3, 2018

The difference is that CROSS_HOST it totally made-up, but the fact this environment variable should not be used by others is what fixes the problem (i.e. it will be empty instead of containing GNU Make pre-defined architecture identifier).

On the other hand, updating the config dot sub and guess scripts in libbacktrace makes the build system understand it is not asked to cross-compile when --host option is set and contains GNU Make pre-defined MAKE_HOST value of a musl libc based host (which is not known to the config.guess/sub scripts in their 2012 version and it may require more updates in the future when new architectures or libc implementations appear).

The best solution, using something like GNU Autoconf, would make use of proper cross-compilation variables only when needed so this would not happen until someone really tries to cross compile (i.e. --host and --build should not be set when calling ./configure script when one does not intend to cross-compile).

@jsarenik
Copy link
Contributor Author

jsarenik commented Jun 6, 2018

Closing this one. Build works well in #1538

@jsarenik jsarenik closed this Jun 6, 2018
@jsarenik jsarenik deleted the jasan/make_host_2nd branch June 6, 2018 12:44
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

Successfully merging this pull request may close these issues.

3 participants