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

fix build failed on macos #204

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leleliu008
Copy link

close #203

@@ -59,5 +59,9 @@ $(ASM_7z).lo: $(ASM_S)
$(ASM_PROG) $(ASM_OPT) -o $(ASM_7z).o $(ASM_S)
mkdir -p .libs
cp $(ASM_7z).o .libs/
@echo -e "$(7ZIPASMLOFILE)" > $(ASM_7z).lo
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

echo -e is not portable. some shell not support this argument.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do this. the .lo file is needed to build the library in lrzip 0.641. Perhaps the cat command could be used. Mac users have compiled lrzip successfully. The echo -e command allows escape characters to print to the output. Perhaps try hard coding /usr/bin/echo since shells are not guaranteed to work the same way as the bin.

@@ -1,4 +1,4 @@
SUBDIRS = C ASM/x86
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASM/x86 have been handled in C submodule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK who added this. But if you do this, you'll have to make sure there is nothing in the x86 dir that IS needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanna successfully build lrzip 0.641. I don't wanna change too much.

@pete4abw
Copy link
Contributor

pete4abw commented Jul 5, 2021

Generally, people who package lrzip would tailor its settings. The changes you suggest are not appropriate for a wide audience. I would make this a local change for your fork.

@leleliu008
Copy link
Author

the macOS user can't build lrzip 0.641 successfully.
Homebrew/homebrew-core#71422
HomeBrew still using lrzip 0.631 due to this problem.
https://github.com/Homebrew/homebrew-core/blob/master/Formula/lrzip.rb

@pete4abw
Copy link
Contributor

pete4abw commented Jul 5, 2021

Just do ./configure --disable-asm. Removing @echo will break things. I have resolved other homebrew issues although I don't know anything about that build system. Your patch will not solve your issue. --disable-asm will.

@leleliu008
Copy link
Author

leleliu008 commented Jul 6, 2021

I'm not just removing @echo but replacing @echo with printf , you should see Files changed. this patch have been tested on my local mac, It can be successfully built.

@pete4abw
Copy link
Contributor

pete4abw commented Jul 6, 2021

Glad it works for you. Certain shells do not support all echo commands. /usr/bin/echo should work as advertised. I don't think I will adopt it. Better solution is to not have to kludge it at all. But automake assembly support does not support pure asm files. That is where I will put my effort. Removing the SUBDIRS could affect how make dist operates.

martin-g added a commit to martin-g/bioconda-recipes that referenced this pull request Jun 4, 2024
Suggested-at: ckolivas/lrzip#204 (comment)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
martin-g added a commit to martin-g/bioconda-recipes that referenced this pull request Jun 4, 2024
Suggested-at: ckolivas/lrzip#204 (comment)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
martin-g added a commit to martin-g/bioconda-recipes that referenced this pull request Jun 4, 2024
Suggested-at: ckolivas/lrzip#204 (comment)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
mencian pushed a commit to bioconda/bioconda-recipes that referenced this pull request Jun 24, 2024
* abyss & btllib: add linux-aarch64 build

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* lrzip: add linux-aarch64 build

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Update lrzip to 0.651 that supports Linux ARM64

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Run autogen.sh for lrzip

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Add `autoconf` to the build requirements

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Add `automake` to the build requirements

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Add `libtool` to the build requirement

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Pass `--disable-asm` for Mac OSX

Suggested-at: ckolivas/lrzip#204 (comment)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* Disable build for osx

* Bump build number

---------

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
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.

build failed on macOS11.4
2 participants