wanxl: use m68k-linux-gnu-as if available

Message ID 20171012232434.15890-1-kilobyte@angband.pl
State Rejected
Delegated to: David Miller
Headers show
Series
  • wanxl: use m68k-linux-gnu-as if available
Related show

Commit Message

Adam Borowski Oct. 12, 2017, 11:24 p.m.
This fixes build failure on Debian based systems: GNU as is the only m68k
assembler available in the archive (package binutils-m68k-linux-gnu).

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
I have no relevant hardware, thus I can't check whether the built firmware
actually works.  Some opcodes are translated differently, thus it might be
possible that some extra options are required.  Or possibly the assembler
has long since changed -- the prebuilt copy hasn't been updated anywhere
within recorded history.

In any case, I admit I don't really care about this driver -- it's merely
a notorious cause of randconfig failures.

 drivers/net/wan/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

David Miller Oct. 18, 2017, 11:08 a.m. | #1
From: Adam Borowski <kilobyte@angband.pl>
Date: Fri, 13 Oct 2017 01:24:34 +0200

> This fixes build failure on Debian based systems: GNU as is the only m68k
> assembler available in the archive (package binutils-m68k-linux-gnu).
> 
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>

The Kconfig help text for the config option controlling the build
firmware is extremely clear what the requirements are for enabling
that option.

And that is: "It requires as68k, ld68k and hexdump programs."
Adam Borowski Oct. 18, 2017, 11:33 a.m. | #2
On Wed, Oct 18, 2017 at 12:08:12PM +0100, David Miller wrote:
> From: Adam Borowski <kilobyte@angband.pl>
> Date: Fri, 13 Oct 2017 01:24:34 +0200
> 
> > This fixes build failure on Debian based systems: GNU as is the only m68k
> > assembler available in the archive (package binutils-m68k-linux-gnu).
> > 
> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> 
> The Kconfig help text for the config option controlling the build
> firmware is extremely clear what the requirements are for enabling
> that option.
> 
> And that is: "It requires as68k, ld68k and hexdump programs."

Yeah, but as far as I know, as68k is a name of "GNU as"; Debian calls the
same m68k-linux-gnu-as (and in general, $(GNU_TRIPLET)-as for all usual
cross toolchains shipped).  And as Debian derivatives make the majority of
distro installs these days, supporting its naming scheme would be
convenient.

Or, if you believe no one should try to build this firmware from source,
it might be better to annotate it to be excluded from randconfig builds.


Meow!
David Miller Oct. 18, 2017, 12:07 p.m. | #3
From: Adam Borowski <kilobyte@angband.pl>
Date: Wed, 18 Oct 2017 13:33:26 +0200

> On Wed, Oct 18, 2017 at 12:08:12PM +0100, David Miller wrote:
>> From: Adam Borowski <kilobyte@angband.pl>
>> Date: Fri, 13 Oct 2017 01:24:34 +0200
>> 
>> > This fixes build failure on Debian based systems: GNU as is the only m68k
>> > assembler available in the archive (package binutils-m68k-linux-gnu).
>> > 
>> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
>> 
>> The Kconfig help text for the config option controlling the build
>> firmware is extremely clear what the requirements are for enabling
>> that option.
>> 
>> And that is: "It requires as68k, ld68k and hexdump programs."
> 
> Yeah, but as far as I know, as68k is a name of "GNU as"; Debian calls the
> same m68k-linux-gnu-as (and in general, $(GNU_TRIPLET)-as for all usual
> cross toolchains shipped).  And as Debian derivatives make the majority of
> distro installs these days, supporting its naming scheme would be
> convenient.
> 
> Or, if you believe no one should try to build this firmware from source,
> it might be better to annotate it to be excluded from randconfig builds.

We don't even know if whatever "as68k" is would be the same thing
as GNU as and generate the same binaries.

If you really must, somehow make sure the proper tools are available
at build or Kconfig time.
Krzysztof Halasa Oct. 19, 2017, 5:25 p.m. | #4
David Miller <davem@davemloft.net> writes:

> We don't even know if whatever "as68k" is would be the same thing
> as GNU as and generate the same binaries.

It's GNU as, likewise ld68k, though I have no idea if recent versions
would compile/link the firmware (correctly). This is 15+ years old.

I don't have opinion on the patch.
Adam Borowski Oct. 19, 2017, 7:31 p.m. | #5
On Thu, Oct 19, 2017 at 07:25:17PM +0200, Krzysztof Halasa wrote:
> David Miller <davem@davemloft.net> writes:
> 
> > We don't even know if whatever "as68k" is would be the same thing
> > as GNU as and generate the same binaries.
> 
> It's GNU as, likewise ld68k, though I have no idea if recent versions
> would compile/link the firmware (correctly). This is 15+ years old.

Thus, we know that:

* this patch can't possibly make things worse: it merely makes compilation
  succeed on systems (like Debian + derivatives) that name m68k GNU as
  m68k-linux-gnu-as rather than as68k

* modern versions of "rose by any other name" (GNU as + ld) produce an
  output that's not byte-to-byte identical.  This would be understandable
  for a compiler, spells trouble for an assembler.  It's possible the modern
  interpretation is still valid, but if it's not, the current rule to
  rebuild the firmware has bitrotten (needs to pass some options to as?)

> I don't have opinion on the patch.

Do you still have access to such hardware?  Could you test it?

Because two scenarios are possible:
*) the build still works (in which case my patch fixes randconfigs on
   Debian)
*) it does not, and you have no resources to fix it.  In which case, it
   would be better to disable the automatic rule and add a comment "to
   build this firmware, dig an ancient version of as+ld".  Or perhaps
   remove this driver as unmaintaineable.


Meow!

Patch

diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index 73c2326603fc..fa17b18765b0 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -38,14 +38,20 @@  obj-$(CONFIG_SLIC_DS26522)	+= slic_ds26522.o
 clean-files := wanxlfw.inc
 $(obj)/wanxl.o:	$(obj)/wanxlfw.inc
 
+HAVE_GNU_M68K_AS := $(shell command -v m68k-linux-gnu-as 2> /dev/null)
 ifeq ($(CONFIG_WANXL_BUILD_FIRMWARE),y)
 ifeq ($(ARCH),m68k)
   AS68K = $(AS)
   LD68K = $(LD)
+else
+ifdef HAVE_GNU_M68K_AS
+  AS68K = m68k-linux-gnu-as
+  LD68K = m68k-linux-gnu-ld
 else
   AS68K = as68k
   LD68K = ld68k
 endif
+endif
 
 quiet_cmd_build_wanxlfw = BLD FW  $@
       cmd_build_wanxlfw = \