Page MenuHomeWildfire Games

Fix musl build
AbandonedPublic

Authored by voroskoi on Dec 20 2019, 3:20 PM.

Details

Reviewers
None
Summary

Make 0ad build with musl libc.

Test Plan

Compiles on Alpine linux.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

voroskoi created this revision.Dec 20 2019, 3:20 PM
linkmauve added inline comments.
source/third_party/cppformat/format.cpp
435

You can remove the && !defined(__BIONIC__) here, it’s either glibc or bionic, never both.

vladislavbelov added inline comments.
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

This seems to be fixed in D2475.

voroskoi updated this revision to Diff 10673.Dec 20 2019, 4:01 PM

Updated the GLIBC-BIONIC part.

voroskoi marked an inline comment as done.Dec 20 2019, 4:02 PM
voroskoi added inline comments.
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

I do not get it, sorry.

Stan added a subscriber: Stan.Dec 20 2019, 4:11 PM
Stan added inline comments.
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

What that means if we have another patch that fixes it :)

vladislavbelov added inline comments.Dec 20 2019, 4:14 PM
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

I mean the patch https://code.wildfiregames.com/D2475 updates NVTT library. So maybe it's not needed to update the file yet (and do it after update).

Also why not isnanf on Linux? It seems the most suitable function for the const float type.

linkmauve added inline comments.Dec 20 2019, 4:19 PM
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

isnan(), isnanf() and isnanfl() are obsolete since C99 introduced the isnan() macro which depends on the type of its input.

voroskoi added inline comments.Dec 20 2019, 4:44 PM
libraries/source/nvtt/src/src/nvmath/nvmath.h
133

I have seen the linked patch, but I have not seen any change in it related the isnanf issue. nvtt git HEAD still uses isnanf() on linux, so upgrading nvtt will not be enough IMO.

Itms added a subscriber: Itms.Dec 20 2019, 4:46 PM

Like Vlad said, I have just uploaded a NVTT upgrade, so it would be better if you'd test the branch (and I would happily include your patch in my upgrade if it's still needed).

It is available at https://github.com/na-Itms/0ad/tree/nvtt. Please let me know if you have trouble and thanks for your interest!

Itms added a comment.Dec 20 2019, 4:52 PM

@voroskoi Maybe you should send a pull request upstream. There is already this PR by a friend but it looks like the NVTT maintainer doesn't want it, maybe they'd rather like your solution.

libraries/source/nvtt/src/src/nvmath/nvmath.h
133

Whoops we commented at the same time. I will include this patch in the NVTT upgrade then.

I have tried your nvtt branch, it fails:
"nvmath.h:193:16: error: 'isnanf' was not declared in this scope; did you mean 'isNan'?"

Anyway, You are right, I will try to get it upstream. Thanks for the heads up!

Stan added a comment.Dec 20 2019, 5:35 PM

Thanks for the patch. If you apply it on Itms branch does it work?

Stan added a comment.Dec 20 2019, 5:57 PM

Thanks for reporting! :)

Itms added a comment.Dec 28 2019, 6:54 PM

Thanks for your patch, I have included the first part (with the other one you needed) in our upgraded NVTT.

For cppformat, we must upgrade that library (see #3190 and #4148). Your patch doesn't apply there so I'm going to reference this diff in the ticket and I hope to perform that upgrade soon. Is it OK with you if I abandon this diff, since we are not going to include it as-is?

Thanks again for the work.

Sure. Thank You!

In D2491#104180, @Itms wrote:

@voroskoi Maybe you should send a pull request upstream. There is already this PR by a friend but it looks like the NVTT maintainer doesn't want it, maybe they'd rather like your solution.

You have a strange way to behave with your friend then.

Itms abandoned this revision.Dec 29 2019, 11:59 AM
elexis added a subscriber: elexis.Dec 31 2019, 4:19 PM
In D2491#104180, @Itms wrote:

There is already this PR by a friend

While I appreciate the work on the software, calling leper your friend is a punch in the face and I will not go down in history as someone who read it and left it uncommented.

Itms added a comment.Dec 31 2019, 4:45 PM
In D2491#104180, @Itms wrote:

There is already this PR by a friend

While I appreciate the work on the software, calling leper your friend is a punch in the face and I will not go down in history as someone who read it and left it uncommented.

I genuinely apologize. I used friend because I wanted to express that we were competitors without hard feelings, so I tried to use a friendlier term, which was apparently interpreted as sarcasm. My hate of sarcasm is at the heart of the issue so I am really sorry for my poor choice of terms.

I think "competitor" is too harsh, "former friend/colleague" too sad, so I'll just use "leper" next time and stop worrying about double meanings. Sorry again.