Skip to content

Use new pipeline syntax #99

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

Merged
merged 2 commits into from
Oct 22, 2015
Merged

Use new pipeline syntax #99

merged 2 commits into from
Oct 22, 2015

Conversation

malmaud
Copy link
Contributor

@malmaud malmaud commented Oct 22, 2015

I'm not exactly sure why, but switching to the new-style pipeline syntax seems to fix the sporadic problems that have been coming up.

Fixes #98
xref JuliaLang/julia#13506

@@ -77,7 +86,11 @@ function install_brew()
if !isfile(joinpath(brew_prefix,"bin","otool"))
# Download/install packaged install_name_tools
try
run(`curl --location $BOTTLE_SERVER/cctools_bundle.tar.gz` |> `tar xz -C $(joinpath(brew_prefix,"bin"))`)
if use_pipeline
run(pipeline(`curl --location $BOTTLE_SERVER/cctools_bundle.tar.gz`, `tar xz -C $(joinpath(brew_prefix,"bin"))`))
Copy link
Contributor

Choose a reason for hiding this comment

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

at least this one should work without the if because pipeline should be in Compat.jl - the other ones might be tuple destructuring related or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try using Compat and see if the fix still works

@tkelman
Copy link
Contributor

tkelman commented Oct 22, 2015

This smells like a real bug in base, shouldn't run(cmd, (STDIN, STDOUT, DevNull), false, false) work?

@malmaud
Copy link
Contributor Author

malmaud commented Oct 22, 2015

Ya, it should work. Maybe a mistake was introduced into Base when run was rewritten to dispatch to pipeline.

I'd advocate for merging this fix pretty soon anyways though to staunch the bleeding of broken Travis badges across the Julia ecosystem while that's investigated.

@tkelman
Copy link
Contributor

tkelman commented Oct 22, 2015

Agreed

@tkelman
Copy link
Contributor

tkelman commented Oct 22, 2015

@staticfloat can you take a look if you get a chance? though looks safe to merge sooner if you're busy

@tkelman
Copy link
Contributor

tkelman commented Oct 22, 2015

xref #88 which was another bug in this piece of code

@malmaud
Copy link
Contributor Author

malmaud commented Oct 22, 2015

As evidenced by https://travis-ci.org/JuliaWeb/MbedTLS.jl/builds/86868002l, this works on Travis.

staticfloat added a commit that referenced this pull request Oct 22, 2015
@staticfloat staticfloat merged commit 5e9407b into master Oct 22, 2015
@staticfloat
Copy link
Member

Thanks guys. :)

@staticfloat
Copy link
Member

Do we need to make a new release?

@malmaud
Copy link
Contributor Author

malmaud commented Oct 22, 2015

Let's tag it and bag it

@malmaud malmaud deleted the jmm/pipeline branch October 22, 2015 18:25
@mlubin
Copy link

mlubin commented Oct 22, 2015

👍

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 error. Maybe because of missing tap
4 participants