all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64.
@ 2022-12-17 18:12 Simon South
  2022-12-17 18:23 ` Simon South
  2022-12-17 19:00 ` Tobias Geerinckx-Rice via Guix-patches via
  0 siblings, 2 replies; 3+ messages in thread
From: Simon South @ 2022-12-17 18:12 UTC (permalink / raw)
  To: 60166

* gnu/packages/patches/flashrom-fix-building-on-aarch64.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/flashing-tools.scm (flashrom)[source]: Apply it.
---

This patch allows the flashrom ROM-flashing utility to build on AArch64 by
backporting a patch[0] that fixes the project's build system so it identifies
the target architecture correctly.  (Currently, trying to build the package on
aarch64-linux fails immediately with "Target arch is unknown. Aborting.")

I've tested this on x86-64 and AArch64 and everything appears fine.  On x86-64
an identical binary is produced with or without this patch applied.

[0] https://review.coreboot.org/plugins/gitiles/flashrom/+/da6b3b70cb852dd8e9f9e21aef95fa83e7f7ab0d

--
Simon South
simon@simonsouth.net


 gnu/local.mk                                  |  1 +
 gnu/packages/flashing-tools.scm               |  4 +-
 .../flashrom-fix-building-on-aarch64.patch    | 89 +++++++++++++++++++
 3 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/flashrom-fix-building-on-aarch64.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 5b8944f568..56634e090c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1112,6 +1112,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/firebird-riscv64-support-pt1.patch	\
   %D%/packages/patches/firebird-riscv64-support-pt2.patch	\
   %D%/packages/patches/flann-cmake-3.11.patch			\
+  %D%/packages/patches/flashrom-fix-building-on-aarch64.patch	\
   %D%/packages/patches/flatpak-fix-path.patch			\
   %D%/packages/patches/flatpak-unset-gdk-pixbuf-for-sandbox.patch	\
   %D%/packages/patches/fontconfig-cache-ignore-mtime.patch	\
diff --git a/gnu/packages/flashing-tools.scm b/gnu/packages/flashing-tools.scm
index 08300cb860..e7165efe79 100644
--- a/gnu/packages/flashing-tools.scm
+++ b/gnu/packages/flashing-tools.scm
@@ -66,7 +66,9 @@ (define-public flashrom
                     version ".tar.bz2"))
               (sha256
                (base32
-                "0ax4kqnh7kd3z120ypgp73qy1knz47l6qxsqzrfkd97mh5cdky71"))))
+                "0ax4kqnh7kd3z120ypgp73qy1knz47l6qxsqzrfkd97mh5cdky71"))
+              (patches
+               (search-patches "flashrom-fix-building-on-aarch64.patch"))))
     (build-system gnu-build-system)
     (inputs (list dmidecode pciutils libusb libftdi))
     (native-inputs (list pkg-config))
diff --git a/gnu/packages/patches/flashrom-fix-building-on-aarch64.patch b/gnu/packages/patches/flashrom-fix-building-on-aarch64.patch
new file mode 100644
index 0000000000..f5fbb0a111
--- /dev/null
+++ b/gnu/packages/patches/flashrom-fix-building-on-aarch64.patch
@@ -0,0 +1,89 @@
+commit da6b3b70cb852dd8e9f9e21aef95fa83e7f7ab0d
+Author: Pyry Kontio <pyry.kontio@drasa.eu>
+Date:   Mon Jul 6 12:57:35 2020 +0900
+
+    Makefile: Fix building on AArch64 NixOS
+    
+    The parsing of the output of archtest.c produced an unexpected
+    value on AArch64 NixOS. For example, the make variable ARCH was set to:
+    
+    ```
+    bit outside of fd_set selected
+    arm
+    ```
+    
+    This made the arch and OS checks fail.
+    
+    This commit simplifies the parsing, making it more robust.
+    
+    The C files archtest.c, endiantest.c and os.h used to set the
+    TARGET_OS, ARCH and ENDIAN variables, respectively, output
+    the result of the test as the final line, so just extracting
+    the final line and removing double quoting is enough.
+    
+    This commit also fixes a bug with debug_shell lacking escaping
+    single quotes, which prevented using the single quote in the
+    debug_shell calls. It used to work by accident before this fix;
+    the line in the call happened to contain a balanced pair of double
+    quotes and lacked other characters that needed escaping, which
+    didn't break the debug_shell, but this was accidental and very
+    brittle.
+    
+    Signed-off-by: Pyry Kontio <pyry.kontio@drasa.eu>
+    Change-Id: Iaa4477a71e758cf9ecad2c22f3b77bc6508a3510
+    Reviewed-on: https://review.coreboot.org/c/flashrom/+/43140
+    Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
+    Reviewed-by: Angel Pons <th3fanbus@gmail.com>
+
+diff --git a/Makefile b/Makefile
+index f3f7717e..e475cbdb 100644
+--- a/Makefile
++++ b/Makefile
+@@ -83,7 +83,8 @@ dummy_for_make_3_80:=$(shell printf "Build started on %s\n\n" "$$(date)" >$(BUIL
+ 
+ # Provide an easy way to execute a command, print its output to stdout and capture any error message on stderr
+ # in the build details file together with the original stdout output.
+-debug_shell = $(shell export LC_ALL=C ; { echo 'exec: export LC_ALL=C ; { $(1) ; }' >&2;  { $(1) ; } | tee -a $(BUILD_DETAILS_FILE) ; echo >&2 ; } 2>>$(BUILD_DETAILS_FILE))
++debug_shell = $(shell export LC_ALL=C ; { echo 'exec: export LC_ALL=C ; { $(subst ','\'',$(1)) ; }' >&2; \
++    { $(1) ; } | tee -a $(BUILD_DETAILS_FILE) ; echo >&2 ; } 2>>$(BUILD_DETAILS_FILE))
+ 
+ ###############################################################################
+ # General OS-specific settings.
+@@ -106,7 +107,8 @@ endif
+ # IMPORTANT: The following line must be placed before TARGET_OS is ever used
+ # (of course), but should come after any lines setting CC because the line
+ # below uses CC itself.
+-override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"'))
++override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null \
++    | tail -1 | cut -f 2 -d'"'))
+ 
+ ifeq ($(TARGET_OS), Darwin)
+ override CPPFLAGS += -I/opt/local/include -I/usr/local/include
+@@ -490,8 +492,10 @@ endif
+ # IMPORTANT: The following line must be placed before ARCH is ever used
+ # (of course), but should come after any lines setting CC because the line
+ # below uses CC itself.
+-override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"'))
+-override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null | grep -v '^\#'))
++override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null \
++    | tail -1 | cut -f 2 -d'"'))
++override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null \
++    | tail -1))
+ 
+ # Disable the internal programmer on unsupported architectures (everything but x86 and mipsel)
+ ifneq ($(ARCH)-little, $(filter $(ARCH),x86 mips)-$(ENDIAN))
+@@ -1299,12 +1303,12 @@ compiler: featuresavailable
+ 	@printf "Target arch is "
+ 	@# FreeBSD wc will output extraneous whitespace.
+ 	@echo $(ARCH)|wc -w|grep -q '^[[:blank:]]*1[[:blank:]]*$$' ||	\
+-		( echo "unknown. Aborting."; exit 1)
++		( echo "unknown (\"$(ARCH)\"). Aborting."; exit 1)
+ 	@printf "%s\n" '$(ARCH)'
+ 	@printf "Target OS is "
+ 	@# FreeBSD wc will output extraneous whitespace.
+ 	@echo $(TARGET_OS)|wc -w|grep -q '^[[:blank:]]*1[[:blank:]]*$$' ||	\
+-		( echo "unknown. Aborting."; exit 1)
++		( echo "unknown (\"$(TARGET_OS)\"). Aborting."; exit 1)
+ 	@printf "%s\n" '$(TARGET_OS)'
+ ifeq ($(TARGET_OS), libpayload)
+ 	@$(CC) --version 2>&1 | grep -q coreboot || \

base-commit: 810b455013037f44801cf1048c81796e77577962
-- 
2.38.1





^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64.
  2022-12-17 18:12 [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64 Simon South
@ 2022-12-17 18:23 ` Simon South
  2022-12-17 19:00 ` Tobias Geerinckx-Rice via Guix-patches via
  1 sibling, 0 replies; 3+ messages in thread
From: Simon South @ 2022-12-17 18:23 UTC (permalink / raw)
  To: 60166

Simon South <simon@simonsouth.net> writes:
> This patch allows the flashrom ROM-flashing utility to build on AArch64 by
> backporting a patch

To be clear, what's being backported here is an upstream commit, not a
patch from elsewhere.

-- 
Simon South
simon@simonsouth.net




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64.
  2022-12-17 18:12 [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64 Simon South
  2022-12-17 18:23 ` Simon South
@ 2022-12-17 19:00 ` Tobias Geerinckx-Rice via Guix-patches via
  1 sibling, 0 replies; 3+ messages in thread
From: Tobias Geerinckx-Rice via Guix-patches via @ 2022-12-17 19:00 UTC (permalink / raw)
  To: Simon South; +Cc: 60166-done, 60166

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

Hi Simon,

Simon South 写道:
> This patch allows the flashrom ROM-flashing utility to build on 
> AArch64 by
> backporting a patch[0] that fixes the project's build system so 
> it identifies
> the target architecture correctly.

Thanks!  Pushed as f28ca2447c5e2eef1ba6a3a11587380a665b0e26.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-17 19:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-17 18:12 [bug#60166] [PATCH] gnu: flashrom: Fix build on AArch64 Simon South
2022-12-17 18:23 ` Simon South
2022-12-17 19:00 ` Tobias Geerinckx-Rice via Guix-patches via

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.