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 oscpack build fail on various architectures #2174

Merged

Conversation

danstowell
Copy link
Member

(amd64, i386, arm64, powerpc and ppc64el...)

Upstreaming a debian patch authored by James Cowgill.
See https://bugs.debian.org/824581 for information.
The patch was new in debian supercollider package version 1:3.7.0_repack-3

I'm proposing the patch for master. It could potentially go to the 3.7 branch too but I'll assume that 3.8 is soon enough that I don't need to.

(amd64, i386, arm64, powerpc and ppc64el...)

Upstreaming a debian patch authored by James Cowgill.
See https://bugs.debian.org/824581 for information.
The patch was new in debian supercollider package version 1:3.7.0_repack-3
@danstowell danstowell added this to the 3.8 milestone Jun 10, 2016
@danstowell danstowell added the comp: build CMake build system label Jun 10, 2016
@timblechmann
Copy link
Contributor

fwiw, this should go to https://github.com/RossBencina/oscpack

also, there is a PR for arm64 support: RossBencina/oscpack#22

@@ -37,42 +37,17 @@
#ifndef INCLUDED_OSCPACK_OSCTYPES_H
#define INCLUDED_OSCPACK_OSCTYPES_H

#include <stdint.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

stdint.h is c99 and c++11. it is above the minimum requirements of oscpack.

Copy link
Member Author

Choose a reason for hiding this comment

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

good point! OK I was about to push a patch further upstream (as you rightly suggested) but I take it this particular patch is unsuitable for upstreaming. (fine for us tho)

@danstowell
Copy link
Member Author

Thanks. That PR you mention, does it interact with this patch? Sorry it's not clear to me.

@timblechmann
Copy link
Contributor

Thanks. That PR you mention, does it interact with this patch? Sorry it's not clear to me.

i don't know (and after people blamed me that i don't care about the community i stopped caring about the community and won't look into it any further). but my guess is that taking oscpack master with that PR will solve your problems.

@danstowell
Copy link
Member Author

Thanks, understood. The other PR is only about arm64 so unlikely to fix all archs. Hoping someone such as Ross will be able to adjudicate.

@timblechmann
Copy link
Contributor

taking oscpack master

@crucialfelix
Copy link
Member

Should we merge this here ? It is going in external_libraries/oscpack_1_1_0/ so it is kind of rewriting history but it's not a bad thing in this case.

The oscpack pr may not get merged for a while, and we would then have oscpack_1_1_1 or whatever.

@danstowell
Copy link
Member Author

(FYI we've already patched oscpack locally in the past - bd9840e - it's not too unusual.)

@crucialfelix crucialfelix merged commit 4c287d2 into supercollider:master Jun 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants