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

Makefile suggestions #248

Closed
outpaddling opened this issue Apr 24, 2019 · 8 comments
Closed

Makefile suggestions #248

outpaddling opened this issue Apr 24, 2019 · 8 comments

Comments

@outpaddling
Copy link

A few possible enhancements for the Makefile, see patch below.

  1. Using PREFIX ?= instead of prefix := will allow some package managers to supply the install location

  2. Is -std=c++98 still necessary on some platforms?

  3. Can uname -m be safely replaced with uname -p? On FreeBSD, the ports system's ${ARCH} variable matches uname -p, which differs from uname -m in some cases. If this change might break the build on other platforms, I can leave it as a patch in the FreeBSD port.

Thanks,

Jason

--- Makefile.orig	2019-04-17 02:40:25 UTC
+++ Makefile
@@ -21,8 +21,9 @@
 # Makefile for bowtie, bowtie2-build, bowtie2-inspect
 #
 
-prefix := /usr/local
-bindir := $(prefix)/bin
+# Use PREFIX (upper case) provided by many package managers
+PREFIX ?= /usr/local
+bindir := $(PREFIX)/bin
 
 LDLIBS := -lz
 GCC_PREFIX := $(shell dirname `which gcc`)
@@ -30,8 +31,9 @@ GCC_SUFFIX :=
 CC ?= $(GCC_PREFIX)/gcc$(GCC_SUFFIX)
 CPP ?= $(GCC_PREFIX)/g++$(GCC_SUFFIX)
 CXX ?= $(CPP)
-CXXFLAGS += -std=c++98
-ifeq (aarch64,$(shell uname -m))
+# long long is unsupported in c++98
+# CXXFLAGS += -std=c++98
+ifeq (aarch64,$(shell uname -p))
 	CXXFLAGS += -fopenmp-simd
 	CPPFLAGS += -Ithird_party/simde
 endif
@@ -198,13 +200,13 @@ SEARCH_FRAGMENTS := $(wildcard search_*_phase*.c)
 VERSION := $(shell cat VERSION)
 
 BITS := 32
-ifeq (x86_64,$(shell uname -m))
+ifeq (x86_64,$(shell uname -p))
 	BITS := 64
 endif
-ifeq (amd64,$(shell uname -m))
+ifeq (amd64,$(shell uname -p))
 	BITS := 64
 endif
-ifeq (aarch64,$(shell uname -m))
+ifeq (aarch64,$(shell uname -p))
 	BITS := 64
 endif
 # msys will always be 32 bit so look at the cpu arch instead.
@@ -219,7 +221,7 @@ endif
 
 SSE_FLAG := -msse2
 M64_FLAG := -m64
-ifeq (aarch64,$(shell uname -m))
+ifeq (aarch64,$(shell uname -p))
 	SSE_FLAG =
 	M64_FLAG =
 endif
@ch4rr0
Copy link
Collaborator

ch4rr0 commented Jun 20, 2019

Overall, some very good suggestions that will most likely make into our next release. We added the explicit --std=c++98 since users reported that the bowtie2 would fail to compile when using newer standards. This is certainly an issue worth revisting.

Thank you again for the suggestions. I will update this thread with progress on the changes.

@ch4rr0 ch4rr0 self-assigned this Jun 20, 2019
@outpaddling
Copy link
Author

Cool.

FYI, it builds for me with no dialect override in clang 6.0 and GCC 7, both of which default to g++14.
https://releases.llvm.org/6.0.0/tools/clang/docs/ReleaseNotes.html#c-language-changes-in-clang
https://helpmanual.io/man1/gcc-7/

Best,

 JB

@ch4rr0
Copy link
Collaborator

ch4rr0 commented Jan 3, 2020

I've made the suggested changes.

@outpaddling
Copy link
Author

Great, thanks much!

@junaruga
Copy link
Contributor

junaruga commented Jan 7, 2020

@outpaddling your suggested modification is still on the bug_fixes branch (= it's a kind of development, topic or hotfix branch), and it's not merged to master branch yet. I think it's better to keep opening this ticket until you see it will be merged to master branch.

@outpaddling
Copy link
Author

OK, thanks for the heads up.

@outpaddling outpaddling reopened this Jan 7, 2020
@ch4rr0
Copy link
Collaborator

ch4rr0 commented Apr 28, 2020

This change is available in v2.4.0 and later.

@ch4rr0 ch4rr0 closed this as completed Apr 28, 2020
@outpaddling
Copy link
Author

One thing that still hasn't changed as of 2.4.1 is "uname -m" -> "uname -p". Is there a reason to stay with "-m" or was this just an oversight? Thanks.

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

No branches or pull requests

3 participants