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

Pipe stdout and stderr to devnull while doing ldflag checks #141

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

Conversation

ankrgyl
Copy link

@ankrgyl ankrgyl commented Sep 25, 2022

Fixes #48 by simply piping stderr and stdout to /dev/null.

@asottile
Copy link
Owner

can you do the other half of the comment and verify it works on the other platforms? sometimes the compiler is fiddly when piped

@ankrgyl
Copy link
Author

ankrgyl commented Sep 25, 2022

Do you have any tips on how? I wasn't sure whether the CI tested it on the set you care about or not. Apologies if I missed something obvious in the repo/instructions.

@asottile
Copy link
Owner

asottile commented Sep 25, 2022

CI gets linux -- you'll probably need to poke around manually on windows, macos (clang and gcc). we used to test them in CI but those platforms are significantly unstable and slow to test so they were removed

@ankrgyl
Copy link
Author

ankrgyl commented Sep 25, 2022

Ah ok. I don't have a windows machine unfortunately -- happy to leave this in the abyss in case someone else feels compelled to test it further.

@asottile
Copy link
Owner

you can get free VMs from https://modern.ie

@ankrgyl
Copy link
Author

ankrgyl commented Sep 25, 2022

I unfortunately don't have cycles to set that up and manually test at the moment, but will try to come back to it another time.

Thank you so much for producing this library btw, it solves a very real need.

@chayim
Copy link

chayim commented Jul 24, 2023

Picking up this conversation @asottile since it's super interesting to me. Any chance I can contribute CI that helps unblock this? I'm happy to lift something similar to what I did in hiredis-py for [freebsd])(https://github.com/redis/hiredis-py/blob/master/.github/workflows/freebsd.yaml) and OSX and/or Windows WDYT?

Given that the fix uses subprocess.DEVNULL, which under the hood uses os.devull - it's should be portable.

@asottile
Copy link
Owner

no thanks, I have no intention of supporting the other platforms and especially if it means making CI 10-20x slower

@chayim
Copy link

chayim commented Jul 24, 2023

Fair. Is there some other way to get this merged in?

@asottile
Copy link
Owner

have you read the thread including my specific instructions above?

@chayim
Copy link

chayim commented Jul 24, 2023

Indeed. This was part of why I offered the CI change.

@asottile
Copy link
Owner

no, you didn't read because if you did you would have seen:

we used to test them in CI but those platforms are significantly unstable and slow to test so they were removed

and then you wouldn't have suggested CI

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.

ldflag checking can result in spurious errors on the console output
3 participants