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

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@pitkley
Copy link

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.

Don't allocate a pseudo-TTY
This fixes issue #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

This comment has been minimized.

Copy link
Member

japaric commented Dec 16, 2017

Thanks, @pitkley. LGTM.

Let's see what homu thinks.

@homunkulus r+

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Dec 16, 2017

📌 Commit 974d8c6 has been approved by japaric

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Dec 16, 2017

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

japaric pushed a commit that referenced this pull request Dec 16, 2017

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Dec 16, 2017

💔 Test failed - status-travis

@pitkley

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 12, 2018

⌛️ Testing commit 974d8c6 with merge 3163527...

japaric pushed a commit that referenced this pull request Jan 12, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 12, 2018

💔 Test failed - status-travis

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 13, 2018

⌛️ Testing commit 974d8c6 with merge 1090151...

japaric pushed a commit that referenced this pull request Jan 13, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 13, 2018

💔 Test failed - status-travis

japaric pushed a commit that referenced this pull request Jan 23, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 23, 2018

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

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 23, 2018

💔 Test failed - status-travis

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 24, 2018

⌛️ Testing commit 974d8c6 with merge bb6c62f...

japaric pushed a commit that referenced this pull request Jan 24, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 24, 2018

⌛️ Testing commit 974d8c6 with merge c9f65fc...

japaric pushed a commit that referenced this pull request Jan 24, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Jan 24, 2018

💔 Test failed - status-travis

@myagley

This comment has been minimized.

Copy link

myagley commented Feb 15, 2018

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

@homunkulus

This comment has been minimized.

Copy link
Contributor

homunkulus commented Feb 17, 2018

⌛️ Testing commit 974d8c6 with merge f85aca9...

japaric pushed a commit that referenced this pull request Feb 17, 2018

Auto merge of #169 - pitkley:docker-no-pseudo-tty, r=japaric
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

This comment has been minimized.

Copy link
Contributor

homunkulus commented Feb 17, 2018

💔 Test failed - status-travis

@myagley

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Collaborator

Dylan-DPC commented May 14, 2018

@darobs

This comment has been minimized.

Copy link

darobs commented May 18, 2018

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

@Dylan-DPC

This comment has been minimized.

Copy link
Collaborator

Dylan-DPC commented Oct 12, 2018

bors: try

bors bot added a commit that referenced this pull request Oct 12, 2018

@malbarbo

This comment has been minimized.

Copy link
Contributor

malbarbo commented Oct 12, 2018

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

This comment has been minimized.

Copy link
Contributor

bors bot commented Oct 12, 2018

@borsboom

This comment has been minimized.

Copy link

borsboom commented Nov 17, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

Disasm commented Feb 20, 2019

bors try

bors bot added a commit that referenced this pull request Feb 20, 2019

@Disasm Disasm requested a review from rust-embedded/tools Feb 20, 2019

@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Feb 20, 2019

@Disasm

This comment has been minimized.

Copy link
Member

Disasm commented Mar 3, 2019

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

@Disasm Disasm requested a review from ryankurte Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.