configure: luac: autodetect host if not provided

When building the luac contrib, it is built at $(PREFIX)/../bin/$(cc -dumpmachine)-luac

When configuring, if --host is not provided, the script looks for luac at $(PREFIX)/../bin/-luac as $host_alias is empty, failing to lookup luac.

Use (cc -dumpmachine) to look for the LUA byte compiler when host is not provided.

mentioned in merge request !5373 (closed)

changed the description

added MRStatus::NotCompliant label

added MRStatus::Reviewable label and removed MRStatus::NotCompliant label

There is a reason why the host parameter exists. If you put the wrong value there, it's on you, not on VLC's build system.

Yes. However, the problem arises when the host parameter is not set. I believe, it should resort to default in that case.

The logic used in this patch is:

When looking for luac-
  if host parameter is set:
      use the host value
  if it is not set:
      guess the host value 

So, behavior in case of providing wrong host value should still be the same.

The problem cannot arise when host_alias is not set because autoconf's AC_CANONICAL_HOST picks a default value if there is none, and thus host_alias cannot not be set.

Here is a google colab vm reproducer for the issue: https://colab.research.google.com/drive/131cYl4xBDdZCwUK88Li7g6aBJyshiwK7?usp=sharing

AC_CANONICAL_HOST sets the $host variable. So, $host_alias remains empty; according to the docs.

So, we could use $host instead of $host_alias.

However, how autotools guesses the host is different from how we guess the host, ie. cc -dumpmachine in contrib, which may result in different guesses.

For example, in case of an amd64 machine with linux, the outpput is: x86_64-pc-linux-gnu vs x86_64-linux-gnu.

In the first place, would you care to explain why there should be a special case for just luac?! It's not exactly the only tool probed by the configure script.

Well, luac is the only tool that uses host_alias in the configure script. Which is why

You can say, this patch finishes the work of 7d31c85c

added MRStatus::InReview label and removed MRStatus::Reviewable label

added 1 commit

• 72eabacd - configure: luac: autodetect host if not provided

Compare with previous version

changed title from configure: luac: autodetected host if not provided to configure: luac: autodetect host if not provided

When configuring, if --host is not provided, the script looks for luac at $(PREFIX)/../bin/-luac as $host_alias is empty, failing to lookup luac.

Can you provide build logs ? Because that's not the only place luac is detected. This particular code is to bypass the regular checks and use the custom build of VLC if it exists. If not, a system one will be used:

  AC_ARG_VAR([LUAC], [LUA byte compiler])
  AS_IF([test -z "$LUAC"], [
     AC_CHECK_TOOL(LUAC, [luac], [false])
  ])
  AS_IF([test "${LUAC}" = "false"], [
    AC_MSG_ERROR([Could not find the LUA byte compiler.])
  ])
  AS_IF([test -d "${CONTRIB_DIR}" -a -f "${CONTRIB_DIR}/lib/liblua5.4.a" -a `echo|${LUAC} -o - -|od -j 12 -N 2 -t x2|head -n 1|tr -s ' '|cut -d' ' -f2` != 0804], [
    AC_MSG_ERROR([You need 64bit luac when using lua from contrib.])
  ])

BTW this change was introduced in 7d31c85c. The commit should be mentioned. It seems reverting to just using host should do the trick.

On my cross compiled build I get:

host='x86_64-w64-mingw32'
host_alias='x86_64-w64-mingw32'

On a native Linux build:

host='x86_64-pc-linux-gnu'
host_alias=''

So that would be an actual revert of that commit. @tguillem do you remember why you made the change ?

added 1 commit

• 8035011d - configure: luac: autodetect host if not provided

Compare with previous version

The elephant in the room which this is ignoring, is that you really should not use contribs without an explicit --host.

Given contribs use dumpmachine unless the triplet is forced, this should work (exactly) the same here. So the equivalent is that if host_alias is set we use that, otherwise dumpmachine. So the path looks correct to me.

However as pointed out in !5386 (comment 438037) the contrib dir location also should involves this logic. So IMO it should be moved up there.

You are assuming that BUILDCC matches, which itself requires that host matches.

IMO it is much saner to assume that host matches than that BUILDCC matches.

It's always better if the host is the knowingly by the user. As I cross compile all the time, that's a given. However Linux users probably expect to run ./configure with no particular flag to just work. If they build contribs either it's in the proper place or they need to use --with-contrib. At that point I think it's fair to assume they can provide a host as well, and we could just decide to support contribs only when --host is set.

changed milestone to %4.0

added Component::Build system label

added 1 commit

• a3f544cb - configure: luac: use host variable from with-contrib processing

Compare with previous version

IMO we should only test one variant of CONTRIB_DIR, not both. But that is subject to debate, so probably for another MR.

approved this merge request

added MRStatus::Stale label and removed MRStatus::InReview label