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

valkyrie: fix build and add test #502

Closed

Conversation

ilovezfs
Copy link
Contributor

Fixes undeclared identifier errors for "getpid," "usleep", and "getuid."
Also, add a test.

@ilovezfs ilovezfs mentioned this pull request Apr 21, 2016
53 tasks
@@ -14,8 +14,14 @@ class Valkyrie < Formula
depends_on "valgrind"

def install
inreplace "src/utils/vk_utils.h", "#include <iostream>",
"#include <iostream>\n#include <unistd.h>"
Copy link
Member

Choose a reason for hiding this comment

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

Can you please submit this patch to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They've already been told they need to include that file for Solaris, but I'm happy to pile on.

Copy link
Member

Choose a reason for hiding this comment

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

Yeh, it'd be good to submit a patch to them and have a linkable reference.

Fixes undeclared identifier errors for "getpid," "usleep", and "getuid."
Also, add a test.

Reported to https://bugs.kde.org/show_bug.cgi?id=362033
@ilovezfs ilovezfs force-pushed the valkyrie-undeclared-identifiers branch from bb535ec to ec90222 Compare April 21, 2016 12:40
@ilovezfs
Copy link
Contributor Author

@MikeMcQuaid upstream link added

@ilovezfs
Copy link
Contributor Author

@DomT4 Is there any more idiomatic way to insert a line?

@DomT4
Copy link
Member

DomT4 commented Apr 21, 2016

Using inreplace? Not as far as I'm aware, sadly. We've tended to handle such things with patch blocks, but this seems reasonable.

@apjanke apjanke closed this in 4c0d059 Apr 21, 2016
@apjanke
Copy link
Contributor

apjanke commented Apr 21, 2016

👍 Merged.

Sorry if I stepped on your toes here; was looking for a test case for a brew pull modification.

@ilovezfs ilovezfs deleted the valkyrie-undeclared-identifiers branch April 27, 2016 13:15
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants