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

Don't allocate a pseudo-TTY #169

Closed
wants to merge 1 commit into from

Conversation

pitkley
Copy link

@pitkley pitkley commented Dec 10, 2017

This fixes issue #52 as far as I can tell.

Since documentation on the Docker -t flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have confirmed that echo hello | cross run works. Furthermore, this enables me to run cross build in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: the input device is not a TTY.

This fixes issue cross-rs#52 as far as I can tell.

Since documentation on the `-t` flag is rather sparse, I am not sure if
this has any negative implications on existing usage of cross.
@japaric
Copy link
Contributor

japaric commented Dec 16, 2017

Thanks, @pitkley. LGTM.

Let's see what homu thinks.

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 974d8c6 has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge 617b9bd...

japaric pushed a commit that referenced this pull request Dec 16, 2017
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

@pitkley
Copy link
Author

pitkley commented Jan 12, 2018

Sorry that I forgot about this PR. Unfortunately, I can't make anything out of the failed architectures. Are those failures unrelated/flaky, or is this something caused by this PR?

@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge 3163527...

japaric pushed a commit that referenced this pull request Jan 12, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge 1090151...

japaric pushed a commit that referenced this pull request Jan 13, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

japaric pushed a commit that referenced this pull request Jan 23, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge 81a7d40...

@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge bb6c62f...

japaric pushed a commit that referenced this pull request Jan 24, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge c9f65fc...

japaric pushed a commit that referenced this pull request Jan 24, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

@myagley
Copy link

myagley commented Feb 15, 2018

Hello. Is there any update on the status of this PR? Thanks!

@homunkulus
Copy link
Contributor

⌛ Testing commit 974d8c6 with merge f85aca9...

japaric pushed a commit that referenced this pull request Feb 17, 2018
Don't allocate a pseudo-TTY

This fixes issue #52 as far as I can tell.

Since documentation on the Docker `-t` flag is rather sparse, I am not sure if this has any negative implications on existing usage of cross.

From my arguably limited tests, I have [confirmed](https://gist.github.com/pitkley/c581f612225688937cc8b7f3a7deff9a) that `echo hello | cross run` works. Furthermore, this enables me to run `cross build` in a Gitlab CI job, which otherwise also failed with the error mentioned in #52: `the input device is not a TTY`.
@homunkulus
Copy link
Contributor

💔 Test failed - status-travis

@myagley
Copy link

myagley commented Apr 30, 2018

Is it possible to please get this merged? I can't tell from the test output what is causing the failures?

@darobs
Copy link

darobs commented May 14, 2018

It would be really nice to run cross in a CI job, is it possible to get this PR merged?

@Dylan-DPC-zz
Copy link

@homunkulus try

@darobs
Copy link

darobs commented May 18, 2018

Thank you @Dylan-DPC - Any insight as to what's happening during the test?

@Dylan-DPC-zz
Copy link

bors: try

bors bot added a commit that referenced this pull request Oct 12, 2018
@malbarbo
Copy link
Contributor

We lost color output if we don't allocate a tty. See for example https://travis-ci.org/rust-embedded/cross/jobs/440652090#L862

@bors
Copy link
Contributor

bors bot commented Oct 12, 2018

try

Build failed

@borsboom
Copy link

This also hit me trying to build using cross using Azure Pipelines, for the same reason as #52 (comment).

Could this PR be modified to use atty to detect whether it's running on a TTY, and use that to decide whether to pass through docker -t?

@ndusart
Copy link

ndusart commented Dec 11, 2018

Any chance this PR could be merged ?

It's the only blocking step before being able to use cross in our CI jobs...

@Disasm
Copy link

Disasm commented Feb 20, 2019

bors try

bors bot added a commit that referenced this pull request Feb 20, 2019
@Disasm Disasm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Feb 20, 2019
@Disasm Disasm requested a review from a team February 20, 2019 18:11
@bors
Copy link
Contributor

bors bot commented Feb 20, 2019

try

Build failed

@Disasm
Copy link

Disasm commented Mar 3, 2019

@rust-embedded/tools Could someone help with this?

@Disasm Disasm requested a review from ryankurte March 3, 2019 08:39
@dignifiedquire
Copy link

any update on this, this is blocking me from using cross on CI

@walfie
Copy link

walfie commented May 10, 2019

@roblourens
Copy link

Sorry to generate more noise on this, but what can we do to help this PR get merged? It's not clear to me whether the CI issues are legit? I would also really like to use this with Azure Pipelines.

Alternatively does anyone know how to get Azure Pipelines to allocate a TTY for a script?

@reitermarkus
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Sep 1, 2019
@bors
Copy link
Contributor

bors bot commented Sep 1, 2019

try

Build failed

@bors bors bot closed this in 13500f7 Sep 1, 2019
@pitkley pitkley deleted the docker-no-pseudo-tty branch September 1, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.