Smalltalk › Squeak › Squeak VM

[OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

56 messages Options

123

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

Hi Vincent,

This still isn't working because
sqFileStdioHandlesIntoFile_WithHandle_IsWritable() doesn't take SQFile
by reference (arrays are just pointers to the first element in C, which
is why the array of SQFile structures doesn't need to be passed by
reference explicitly):

sqFileStdioHandlesIntoFile_WithHandle_IsWritable(SQFile *file, HANDLE handle, int isWritable) {
        file->sessionID = thisSession;
        file->file = handle;
        file->writable = isWritable;
        file->lastOp = 0; /* unused on win32 */
        file->isStdioStream = isFileHandleATTY(handle);
        AddHandleToTable(win32Files, handle);
}


sqFileStdioHandlesInto(SQFile files[3])
{
        sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[0], GetStdHandle(STD_INPUT_HANDLE), false);
        sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[1], GetStdHandle(STD_OUTPUT_HANDLE), true);
        sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[2], GetStdHandle(STD_ERROR_HANDLE), true);

        return 7;
}

I think that gets the basic changes you wanted done. But it still
leaves stdio in cygwin terminals broken. After a bit more reading it
looks like cygwin terminals and Windows ReadConsole() and WriteConsole()
functions are fundamentally incompatible.

I don't understand what ReadConsole() and WriteConsole() provide in
terms of benefits over FILE* streams, so I can't really comment on
the cost of replacing them.

Eliot, what are your thoughts about moving the stdio functionlity to use
FILE* streams instead of HANDLE streams?

I think that would allow the VM to use fread(), feof(), and fwrite() to
read and write stdio to terminals (cygwin), consoles (windows), pipes
and regular files.

Another option may be to leave the existing console functionality and
use the connectToFileDescriptor() routine to separately open the stdio
streams as FILE* streams. The image can then decide how it wants to
interact with stdio.

(we should probably move this conversation to vm-dev and out of this
particular PR)

Thanks,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

I forgot that structs are directly stored in the array without pointer to them. I fixed it.
What do you mean by broken? Do you have an example I can reproduce?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

@VincentBlondeau pushed 1 commit.

2a95124 An array of structs have to be passed as reference...

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

@VincentBlondeau pushed 1 commit.

a64662f The arguments are typed in C

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Vincent & Eliot,

What do you mean by broken? Do you have an example I can reproduce?

Sure - if you start PharoConsole.exe from within a cygwin (mintty)
terminal and remove the #isWin32 block from Stdio
class>>standardIOStreamNamed:forWrite: so that you are actually using
stdout, and then attempt to write something to stdout you'll get a
primitive failure. The same sequence works from a dos box (cmd.exe).

That's because the read and write primitives rely on the Windows
ReadConsole() and WriteConsole() functions, which only work in a dos
box, not in mintty.

When we started these modifications I assumed that it would be possible
to have I/O that worked in both cygwin (mintty) and a dos box (cmd.exe).
However on further reading it looks like MingW programs (which use
either Windows functions such as ReadConsole() and WriteConsole() or
the MS posix implementation) will only work properly in a dos box. If
you want proper mintty support you need to use the cygwin implementation
of the posix functions.

As an example, attempting to use the MingW/MS fread() function and
signal EOF:

In a dos box, typing Control-Z correctly signals EOF.
In a cygwin terminal (mintty), Control-D is never passed to the
program, so EOF is never true. (If you then type Control-C to
terminate the program, sometimes the terminal then consumes the
earlier Control-D and logs you out :-().

Someone please explain how I am wrong and how to get stdio working on
both dos boxes and cygwin terminals...

Thanks,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Eliot Miranda-2

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

Hi Alistair,

On Apr 26, 2018, at 12:35 PM, akgrant43 <[hidden email]> wrote:

Hi Vincent & Eliot,

What do you mean by broken? Do you have an example I can reproduce?

Sure - if you start PharoConsole.exe from within a cygwin (mintty)
terminal and remove the #isWin32 block from Stdio
class>>standardIOStreamNamed:forWrite: so that you are actually using
stdout, and then attempt to write something to stdout you'll get a
primitive failure. The same sequence works from a dos box (cmd.exe).

That's because the read and write primitives rely on the Windows
ReadConsole() and WriteConsole() functions, which only work in a dos
box, not in mintty.

When we started these modifications I assumed that it would be possible
to have I/O that worked in both cygwin (mintty) and a dos box (cmd.exe).
However on further reading it looks like MingW programs (which use
either Windows functions such as ReadConsole() and WriteConsole() or
the MS posix implementation) will only work properly in a dos box. If
you want proper mintty support you need to use the cygwin implementation
of the posix functions.

As an example, attempting to use the MingW/MS fread() function and
signal EOF:

In a dos box, typing Control-Z correctly signals EOF.

In a cygwin terminal (mintty), Control-D is never passed to the
program, so EOF is never true. (If you then type Control-C to
terminate the program, sometimes the terminal then consumes the
earlier Control-D and logs you out :-().

Someone please explain how I am wrong and how to get stdio working on
both dos boxes and cygwin terminals...

Ugh. I didn't realize one couldn't use the same code in both :-(. I guess we have to test to find out what the context is and use dread in one and ReadConsole in the other. But I don't get it. IIRC, it used to be the case that ReadConsole/WriteConsole worked in both.

Thanks,
Alistair

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@akgrant43 in #254: Hi Vincent \u0026 Eliot,\r\n\r\n\u003e What do you mean by broken? Do you have an example I can reproduce?\r\n\r\nSure - if you start PharoConsole.exe from within a cygwin (mintty) \r\nterminal and remove the #isWin32 block from Stdio \r\nclass\u003e\u003estandardIOStreamNamed:forWrite: so that you are actually using \r\nstdout, and then attempt to write something to stdout you'll get a \r\nprimitive failure. The same sequence works from a dos box (cmd.exe).\r\n\r\nThat's because the read and write primitives rely on the Windows \r\nReadConsole() and WriteConsole() functions, which only work in a dos \r\nbox, not in mintty.\r\n\r\nWhen we started these modifications I assumed that it would be possible \r\nto have I/O that worked in both cygwin (mintty) and a dos box (cmd.exe). \r\nHowever on further reading it looks like MingW programs (which use \r\neither Windows functions such as ReadConsole() and WriteConsole() or \r\nthe MS posix implementation) will only work properly in a dos box. If \r\nyou want proper mintty support you need to use the cygwin implementation \r\nof the posix functions.\r\n\r\nAs an example, attempting to use the MingW/MS fread() function and \r\nsignal EOF:\r\n\r\n- In a dos box, typing Control-Z correctly signals EOF.\r\n- In a cygwin terminal (mintty), Control-D is never passed to the \r\n program, so EOF is never true. (If you then type Control-C to \r\n terminate the program, sometimes the terminal then consumes the \r\n earlier Control-D and logs you out :-().\r\n\r\nSomeone please explain how I am wrong and how to get stdio working on \r\nboth dos boxes and cygwin terminals...\r\n\r\nThanks,\r\nAlistair\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/254#issuecomment-384764181"}}}</script>

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Vincent & Eliot,

Ugh. I didn't realize one couldn't use the same code in both :-(. I guess we have to test to find out what the context is and use dread in one and ReadConsole in the other.

I think it is worse than that. If I use the following test code in a
cygwin terminal:

#include <stdio.h>
#include <stdlib.h>


int main()
{
char	buf[1024];
char	*bufp;
int	cread;
int	count = 0;

bufp = &buf[0];
do {
    cread = fread(bufp, 1, 1, stdin);
    count += cread;
    bufp += cread;
    if (count > 16) break; }
while (cread > 0);
buf[count] = 0;

if (! feof(stdin))
    fprintf(stderr, "Error, not at end of file\n");

printf("--\n");
printf("%s\n", buf);
printf("--\n");
printf("Read %d characters\n", count);
exit(0);
}

And compile it with mingw:

$ i686-w64-mingw32-gcc -m32 consolestdio.c

It won't recognise EOF (Control-D).

$ ./a.exe
hello world
<Ctrl-D>
asdfasdfasdfasdfasdf
--
hello world

asdf
--
Read 17 characters
Error, not at end of file

If I compile it with gcc:

$ gcc -m32 consolestdio.c

It works as expected:

$ ./a.exe
hello world
<Ctrl-D>
--
hello world

--
Read 12 characters

So the best option I see for supporting both cygwin terminals and dos
boxes is to make our own dll that wraps around the cygwin1.dll functions
and to load it as required. Although I'm a bit concerned that I haven't
found any mention of doing this elsewhere (and it's a lot of work for
one specific use case).

But I don't get it. IIRC, it used to be the case that ReadConsole/WriteConsole worked in both.

I'd still like to be proved wrong...

Thanks,
Alistair

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Ben Coman

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

On 27 April 2018 at 15:16, akgrant43 <[hidden email]> wrote:

Hi Vincent & Eliot,

Ugh. I didn't realize one couldn't use the same code in both :-(. I guess we have to test to find out what the context is and use dread in one and ReadConsole in the other.

I think it is worse than that. If I use the following test code in a
cygwin terminal:
#include <stdio.h>
#include <stdlib.h>


int main()
{
char	buf[1024];
char	*bufp;
int	cread;
int	count = 0;

bufp = &buf[0];
do {
    cread = fread(bufp, 1, 1, stdin);
    count += cread;
    bufp += cread;
    if (count > 16) break; }
while (cread > 0);
buf[count] = 0;

if (! feof(stdin))
    fprintf(stderr, "Error, not at end of file\n");

printf("--\n");
printf("%s\n", buf);
printf("--\n");
printf("Read %d characters\n", count);
exit(0);
}
And compile it with mingw:
$ i686-w64-mingw32-gcc -m32 consolestdio.c
It won't recognise EOF (Control-D).

Try Ctrl-Z

"In a non-Cygwin Windows program, Ctrl-Z on input triggers an end-of-file condition."

https://stackoverflow.com/questions/39327818/reading-till-eof-in-java-on-cygwin-on-windows-10

cheers -ben

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Ben,

Try Ctrl-Z
"In a non-Cygwin Windows program, Ctrl-Z on input triggers an end-of-file condition."

I had tried this, and since it's bash it just stopped the program, as expected.

I'd also looked at the page you referenced, but on reading it again thought I'd try unsetting the susp key:

stty susp ''

And sure enough, running the mingw version works this time:

$ ./mingw.exe
asdf<Enter>
<Ctrl-Z><Enter>
--
asdf

--
Read 5 characters

While it is a workaround, I'm not sure that asking users to modify the terminal settings and remember to use Ctrl-Z instead of Ctrl-D for Pharo in bash is going to be practical.

What do you think?

Just for LOLs, the following does work:

$ cat | vm/PharoConsole Pharo7.0.image eval StdioStreamTest manualStdinTest | tee /dev/null
asdf<Enter>
<Ctrl-D>

It doesn't work in headless mode, I haven't figured out why yet.

Cheers,
Alistair

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

Ben Coman

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

On 27 April 2018 at 19:18, akgrant43 <[hidden email]> wrote:

While it is a workaround, I'm not sure that asking users to modify the terminal settings and remember to use Ctrl-Z instead of Ctrl-D for Pharo in bash is going to be practical.
What do you think?

Probably not. It just helps characterize the problem better.

cheers -ben

alistairgrant

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Eliot & Vincent,

On 27 April 2018 at 09:16, akgrant43 <[hidden email]> wrote:
>
>
>
> > But I don't get it. IIRC, it used to be the case that ReadConsole/WriteConsole worked in both.
>
> I'd still like to be proved wrong...

This thread has been quiet for a few days, so I guess my wishes aren't
met and there isn't a way to have a program work in both Windows
Consoles and cygwin terminals.

One of the articles forwarded by Ben suggested the workaround is to have
a .com version of the program, which does all the work if the command
line arguments only require console i/o. If a gui is required, it
relaunches the .exe version which then opens the windows. We could
extend that to also check for cygwin and relaunch a cygwin exe.
However I think that is effectively adding another platform to support,
which is more effort than I want to put in.

Assuming all of the above, I think we might as well (reluctantly) rip
out the code that tests for cygwin terminals and just leave Vincent's
primitives in. This will at least allow the image to properly determine
whether it can write to the console or if it should create files
instead.

What do you think?

Thanks,
Alistair

alistairgrant

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

Actually, the primitive needs to be an enumerated value indicating one of:

- no console (windows only)

- console / terminal

- pipe

- file

Cheers,
Alistair
(on phone)

On Tue., 1 May 2018, 07:56 Alistair Grant, <[hidden email]> wrote:

Hi Eliot & Vincent,

On 27 April 2018 at 09:16, akgrant43 <[hidden email]> wrote:
>
>
>
> > But I don't get it. IIRC, it used to be the case that ReadConsole/WriteConsole worked in both.
>
> I'd still like to be proved wrong...

This thread has been quiet for a few days, so I guess my wishes aren't
met and there isn't a way to have a program work in both Windows
Consoles and cygwin terminals.

One of the articles forwarded by Ben suggested the workaround is to have
a .com version of the program, which does all the work if the command
line arguments only require console i/o. If a gui is required, it
relaunches the .exe version which then opens the windows. We could
extend that to also check for cygwin and relaunch a cygwin exe.
However I think that is effectively adding another platform to support,
which is more effort than I want to put in.

Assuming all of the above, I think we might as well (reluctantly) rip
out the code that tests for cygwin terminals and just leave Vincent's
primitives in. This will at least allow the image to properly determine
whether it can write to the console or if it should create files
instead.

What do you think?

Thanks,
Alistair

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

So this is real goodness. Can we not address the issues and get this in? Right now as I understand it we know how to discover both a Cygwin console and a Windows Console, but the two require separate approaches. Can we not add both of them? As far as the end-of-file issue dgoes I would say that is not a deal breaker. If I were implementing a REPL I would want EOF to cause the system to exit cleanly but I would also live with having to issue a proper quit command.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Eliot and Vincent,

(I only have phone access, so apologies for typos, formatting, etc.)

Unless I'm misunderstanding the situation, we can't easily add support
for both cygwin terminals and windows consoles.

We're compiling and linking with mingw, which means we have access to
the windows consoles directly (through ReadConsole() and
WriteConsole()). If we want to access the cygwin terminals properly we
need to use the posix io functions in cygwin1.dll (not the MS supplied
versions of the posix io functions).

I guess it would be possible to write a wrapper DLL around the posix io
functions in cygwin1.dll and load that if we recognise that we are in a
cygwin terminal. But that is obviously a fair bit of effort (not
something that I'm interested in doing in the short or medium term).

Given your acceptance of not recognising EOF correctly in a cygwin
terminal I would extend my previous suggestion that we modify Vincent's
primitiveIsFileDescriptorATTY to return an enumerated value:

0 - no console (windows only)
1 - normal terminal (unix terminal / windows console)
2 - pipe
3 - file
4 - cygwin terminal (windows only)

What do you think?

Cheers,
Alistair

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Alistair,

I agree we shouldn't include cygwin1.dll.  But the existence of cygwin1.dll is proof that the relevant code can be implemented above Win32.  I do know that one can read from cygwin terminals; I was doing that regularly with the old VM.  So the issue is simply identifying a cygwin console as a tty.  We can put reading/writing from/to it to one side and first focus on identification.  I'm happy for you to extend primitiveIsFileDescriptorATTY as you suggest.  What I think we need, though, is to be able to identify either a Windows console or a cygwin console independently of cygwin1.dll (even if we have to copy some liberally licensed code from it).

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Great, I think we're in agreement.

Vincent, what do you think?

Cheers,
Alistair

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Sounds good! The only thing is I can check the result only under windows. So, I don't know which values to return for Linux if the terminal is not a tty. Should I return 2 or 3? And how do I know that is a file or a pipe?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Vincent,

Alistair defined the values earlier in the thread:

0 - no console (windows only)
1 - normal terminal (unix terminal / windows console)
2 - pipe
3 - file
4 - cygwin terminal (windows only)

I expect it'll be fine for you to extend the windows code but leave the Unix code for Alistair to implement.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

Hi Vincent,

I assumed that for Windows you could already distinguish between pipes and normal files since the pty check uses the pipe name as part of its check. If not...

The fstat() st_mode field has S_IFIFO which indicates a first-in first-out (pipe) file.

Some of the documentation I saw said that S_IFIFO is for named pipes, I don't know what it will return for stdin being a pipe.

fstat() takes a file descriptor, so you'll need to convert a HANDLE to a fd.

If you can get Windows working I'm happy to take care of the Unix side.

Cheers,
Alistair

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

David T Lewis

Re: [OpenSmalltalk/opensmalltalk-vm] Add accurate check for console stdio + add associated primitive (#254)

In reply to this post by David T Lewis

@VincentBlondeau pushed 2 commits.

a2cb286 changes in platforms
3b48a7a Merge branch 'Cog' into addStdoutIsConsolePrimitive

—
You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

123