Aha.  It would be great to say all that in the source code!!  It’s very 
non-obvious that you not want people ever to construct a CGId on Windows.  
After all, it has a newtype definition!

Could you declare it differently?
    data CGId   -- No constructors

Also if so, why does the Windows-specific foreign import of 
c_runInteractiveProcess (lin 440-is) pas a Ptr CGId? You’d just told me that we 
can never create one.

Also,

·         It’d help to comment the #else on line 456 as being “else if not 
windows”

·         The #endifs on line 546 or thereabouts are mis-labelled.  at the 
moment the second says “GLASGOW_HASKELL” but actually it’s the first

I have no opinion about the best solution; I’d just like it to compile and 
preferably warning free since that is our default policy.  Or add a –Wwarn at 
the top.

thanks

Simon


From: [email protected] [mailto:[email protected]] On Behalf Of 
Michael Snoyman
Sent: 02 November 2015 13:42
To: Simon Peyton Jones
Cc: [email protected]
Subject: Re: process library broken on Windows

That's the goal; it's a feature that does not work on Windows, only on 
non-Windows systems (setuid/setgid for a child process). For POSIX systems, 
CGid is exported from base, and can be used. On Windows, the data type is 
present to give the same signature, but the constructor itself is not exported 
to prevent using the feature. An argument could be made for other approaches:

1. Expose the constructor, allowing users to set a value, and that value will 
be ignored
2. Make the fields themselves conditional on the OS being used

I don't think there's a strong argument in any direction for this.

On Mon, Nov 2, 2015 at 5:37 AM, Simon Peyton Jones 
<[email protected]<mailto:[email protected]>> wrote:
I’m puzzled.   Internals.hs defines a newtype

newtype CGid = CGid Word32

A value of this type is needed to fill in the child_group field of 
CreateProcess.  If you don’t export it, you could never initialise this field 
to anything other than Nothing, so why do you have it?

Looks to me as if the warning has nailed a real bug

Simon

From: [email protected]<mailto:[email protected]> 
[mailto:[email protected]<mailto:[email protected]>] On Behalf 
Of Michael Snoyman
Sent: 02 November 2015 13:34
To: Simon Peyton Jones
Cc: [email protected]<mailto:[email protected]>
Subject: Re: process library broken on Windows

I didn't read closely enough: I see now that it's a warning, not an error. I 
initially didn't export that constructor since it's only present on Windows for 
API compatibility, but will never be used. Since this is just the internals 
module, I can export it, but my preference would in fact be to leave it as-is 
with the warning. Two alternatives:

1. Create a new hidden module that creates and exports the type constructor, 
just to hide the warning. I'm -1 on that, since that's extra compile time 
everyone has to endure just for warning avoidance.
2. base could export CGid for Windows (currently, it does not).

On Mon, Nov 2, 2015 at 5:17 AM, Michael Snoyman 
<[email protected]<mailto:[email protected]>> wrote:
I'll look into this. I just made a new release of process, and was certain I 
tested on Windows, but perhaps something changed between that commit and 
release.

On Mon, Nov 2, 2015 at 5:15 AM, Simon Peyton Jones 
<[email protected]<mailto:[email protected]>> wrote:
I’m getting this on HEAD in te ‘

libraries\process\System\Process\Internals.hs:106:16: warning:

    Defined but not used: data constructor ‘CGid’
Indeed it looks as if CGId(..) should be exported, else createProcess is 
unusuable.  This looks like the right change.  Would someone like to check and 
make the change
Simon

diff --git a/System/Process/Internals.hs b/System/Process/Internals.hs

index 5575ac4..3e23ad5 100644

--- a/System/Process/Internals.hs

+++ b/System/Process/Internals.hs

@@ -37,6 +37,8 @@ module System.Process.Internals (

#if !defined(mingw32_HOST_OS) && !defined(__MINGW32__)

     pPrPr_disableITimers, c_execvpe,

     ignoreSignal, defaultSignal,

+#else

+    CGid(..), GroupID, UserID,

#endif

     withFilePathException, withCEnvironment,

     translate,

_______________________________________________
ghc-devs mailing list
[email protected]<mailto:[email protected]>
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs<https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fmail.haskell.org%2fcgi-bin%2fmailman%2flistinfo%2fghc-devs&data=01%7c01%7csimonpj%40064d.mgd.microsoft.com%7cdd25a0d693de489171bb08d2e38a505d%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=YdqpMC5wr2K6oUOw9WImRGpSl6EsV8zQyAK%2ba26oF9M%3d>



_______________________________________________
ghc-devs mailing list
[email protected]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Reply via email to