ericwj / GoWindows

This repository contains infrastructure to test parts of go on Windows against .NET and Win32.
0 stars 1 forks source link

spec for Go app to invoke path/filepath APIs #1

Open networkimprov opened 4 years ago

networkimprov commented 4 years ago

Input: an API name & required arguments. From command line args or stdin? Test input: a flag and output or error string; Go API not called.

Output: a pathname. To stdout?

Error: a Windows error code and associated Go string. To stderr?

Analysis: comparison of output with expected result lives in external script?

Process: do you expect to modify this Go code?

Note: I'm not sure that this effort will yield any bugfixes in Go. The Windows maintainer is not a Google employee, and has opposed bug reports and proposals on grounds that they could break existing programs, without giving real-world examples of such programs. The Go team is reluctant to overrule him. There are few active contributors to the Windows port to advocate fixes. We may only be able to get something added to x/sys/windows. Worst case, we'll have to publish a third-party package.

ericwj commented 4 years ago

Its fine to use stdin and stdout.

The inputs and outputs would systematically follow the go API arguments and the result type it returns. Program.cs has XML comments that provides a sort of spec. If I am correct assuming that you use VS Code to write go - as I have heard that is the defacto standard IDE with very good support for the language, it will mostly automatically be able to install the C# plugins. After that, if you hover over class Program you will get quite neat text to read and that works for anything annotated with XML comments (///)

So the code I write to use this API will just call go.IsAbs("path") and get a bool back as if I was writing go. If there is an error in parsing input or output then I'm fine having it simply crash on the C# side or if the client go program exits with an exit code other than zero.

This way I can also just write regular XUnit unit tests that are straightforward to port.

I certainly do hope to be able to produce bug fixes, since I will show that standard calls will always fail in certain situations -- the changes I will introduce well they might not yield a path like novice Windows users would be wrongfully expecting (C:...) but they will not crash programs that crash today.

ericwj commented 4 years ago

Let me type that spec over here.

So to provide a few examples:

ericwj commented 4 years ago

We will finally for WalkFunc define names that are substituted for the delegates they represent.

It is probably enough to have just one or two, one that counts the number of results and writes that as the result to the call and one that simply dumps the full path of every item.

This serializes as

(blank line)
Walk
root: C:\
walkFn: DefaultWalk
end

Which immediately writes:

(blank line)
Walk

then outputs any output implemented by the actual WalkFunc with the name WalkFunc:

WalkFunc: C:\pagefile.sys
WalkFunc: C:\hiberfil.sys
(etc)

etc, then optionally summaries if defined, e.g. for CountingWalk (see below)

WalkFunc: 12345678

then result value of the call

result: #Access denied#5#
end
ericwj commented 4 years ago

Lets change the prefix and suffix for commands.

From prefix commandname and suffix end. To prefix commandname: begins and suffix commandname: ends.

networkimprov commented 4 years ago

Since you want name/value pairs, let's format input and output as JSON objects. That's easy to do in Go. I don't want to re-invent parsing.

Any command taking a variable list of arguments would be formatted as an array, even if a single argument is supplied. Error code and string would be separate fields. Strings cannot be null in Go, only empty. Arrays can be null or empty.

Is WalkFunc an API to call on each node in a filesystem tree? If not, what does it provide the calling script?

ericwj commented 4 years ago

Well JSON is messy for humans and the parsing needed is very basic.

Plus, JSON breaks down for WalkFunc - it is supposed to write to the console in a streaming fashion.

No WalkFunc is the argument you give to Walk. I just call Walk("C:\\", "DefaultWalk"). You then call Walk("C:\\", DefaultWalk) after defining func DefaultWalk(string, os.FileInfo, error): error yourself in code. Without JSON it is just able to call fmt.Print which I read in a streaming fashion. That is much, much harder to do with JSON parsing.

ericwj commented 4 years ago

Plus there can be times some calls take very long or the process crashes, in which case I am not sure I will receive end of stream or what will happen, so I might have to go async and define timeouts, which also complicates parsing if it is not purely on a line-by-line basis.

networkimprov commented 4 years ago

So the point of Walk is to stream FileInfo objects from a dir? Do you need fields in them besides Name() & IsDir()?

I still don't follow the uses of Walk. Pls tell me what it does and why.

I can stream separate JSON objects or strings and you can decode them a line at a time.

If you want human-writable inputs to your script, you can parse them with PS and generate JSON for me. I am just an AI, sorry.

ericwj commented 4 years ago

Mmm, yeah look here. It does what you expect, plus it eats error codes while doing it and sorts each directories contents by name and calls WalkFunc for each directory or file 'in lexical order'.

ericwj commented 4 years ago

At this time I don't see use for Size, Mode and ModTime or Sys, no. Whether these are OK is another API's problem.

Even IsDir isn't quite immediately required. I guess what I will want to do is create a directory structure and check that the WalkFunc yields a list of names that matches some expected list of names. I can pretty much expect without even checking that something is a directory that it actually is according to the os.FileInfo if the list contains full paths to files under it. Any list not matching the expected list will be shot down as a failure anyway.

networkimprov commented 4 years ago

OK, working on first draft...

ericwj commented 4 years ago

I am not sure C# will give me tuple element names for type parameters. Can we standardize on a result named

(string result, error err)

for any result that has tuple type (string, error)?

That is for Abs, EvalSymLinks and Rel.

Both (bool, error) and ([]string, error) occur only once so these can be named as specified in go or also be called (result, err) as you see fit.

ericwj commented 4 years ago

And just to be explicit, with JSON it'd become.

Call IsAbs<(string path), (bool result)>:

{ "Name": "IsAbs", "args": { "path": "C:" } }

Response

{ "Name": "IsAbs", "result": true }

Or if the result is a tuple

{ "Name": "Glob", "result": { "matches": [ "foo.txt" ], "err": null } }

or

{ "Name": "Abs", "result": { "result": [ "C:\\path" ], "err": { "Error": "Huh, both!?", "Code": null } } }

Calling Walk would become:

Call:

{ "Name": "Walk", "args": { "root": "C:\\", "walkFn": "DefaultWalk" } }

Immediate response:

{ "Name": "Walk" }

Then a stream of:

{ "path": "C:\foo", "info": { ... }, "err": null }

well assuming path matches info.Name it is duplicate and if it is not, that is not our problem, all of info could be omitted for additional brevity.

And finally:

{ "Name": "Walk", "result": { "Error": "What could this ever be?", "Code": null } }

If it can happen both a result is not empty and the error is not null, then I would prefer receiving err before any possibly lenghty string such that the errors align at the left of the output, usually with a neat { "err": null, ... } if you cannot avoid writing default values.

camelCasing usually I avoid it, I don't care about conventions - it feels like typing sloppily. Here I just matched whatever case go used and the naming of properties that I would define in C#, but I guess the JSON property names will be case-insensitive anyway.

ericwj commented 4 years ago

It would be more efficient to do { "Name": "IsAbs"}{ "path": "C:" }. The C# oneliner will have to parse the whole thing twice because I have to know the name before deserializing the arguments. You pick.

networkimprov commented 4 years ago

Remember, no battle plan survives first contact with the AInemy :-)

Posted a draft; tested on Linux only for the moment. https://github.com/ericwj/GoWindows/tree/master/src/go

networkimprov commented 4 years ago

I'd suggest you parse the user-crafted input into a struct in powershell, and convert it to JSON (PS can do that?) when ready to send to stdout. It's probably best to avoid a C# build step in the go builder instance that will run these tests.

I don't see why you'd need the api name in the output, but could add that. There's no need to nest the input arguments or output error fields in an object, I don't think. The .Errno output could be omitted if zero. Input field names are case insensitive. Output field names could be made lowercase. More input fields will be added for APIs that need them. Any not needed should be omitted in input.

ericwj commented 4 years ago

No its ok like this - not sure why you added path.filepath unless you expect to include other things later at which time that would be a simple addition.

It is awful to type

PS> "{ ""Api"": ""path.filepath.IsAbs"", ""Path"": ""$($PWD.Path.Replace("\", "\\"))"" }" | .\go.exe
{"Errno":0,"Result":true}

or

PS> "{ ""Api"": ""path.filepath.IsAbs"", ""Path"": ""\\\\?\\Volume{607932d3-78af-41c0-9786-dd0177e78a39}"" }" | .\go.exe
{"Errno":0,"Result":false}

but there is the first bug.

and quite involved when having PS do it

Get-Partition -DiskNumber 6 -PartitionNumber 2 | 
select -ExpandProperty AccessPaths | 
where { $_.StartsWith("\\") } | 
foreach { [pscustomobject]@{ Api = "path.filepath.Walk"; Path = $_ } } | 
foreach { "$($_ | ConvertTo-Json)".Replace("`r`n", "") } | 
foreach { $_ | .\go.exe | 
foreach { $_ | ConvertFrom-Json } }

but this API is fine with the volume path.

ericwj commented 4 years ago

Heck here you go with the string manipulations

PS> "{ ""Api"": ""path.filepath.IsAbs"", ""Path"": ""C:"" }" | .\go.exe
{"Errno":0,"Result":false}
ericwj commented 4 years ago

No these API's are OK; they refer to the current working directory on that volume.

ericwj commented 4 years ago

This is absolutely wrong

PS> "{ ""Api"": ""path.filepath.Abs"", ""Path"": ""\\\\?\\."" }" | .\go.exe
{"Errno":0,"Result":"\\?"}

And if the above is OK then this is certainly not

PS> cd Y:\Source
PS> C:
PS> "{ ""Api"": ""path.filepath.Abs"", ""Path"": ""Y:"" }" | .\go.exe
{"Errno":0,"Result":"Y:\\"}
ericwj commented 4 years ago
 "{ ""Api"": ""path.filepath.Walk"", ""Path"": ""\\\\?\\Volume{607932d3-78af-41c0-9786-dd0177e78a39}"" }" | .\go.exe
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x30 pc=0x4dd9fe]

goroutine 1 [running]:
main.walkInfo(0xc0000b6060, 0x30, 0x0, 0x0, 0x53b220, 0xc0000b8420, 0xc000096440, 0x0)
        C:/Users/Eric/Source/Repos/GoWindows/src/go/api.go:58 +0x3e
path/filepath.Walk(0xc0000b6060, 0x30, 0x51f1d0, 0x1, 0x0)
        C:/tools/go/src/path/filepath/path.go:404 +0x71
main.main()
        C:/Users/Eric/Source/Repos/GoWindows/src/go/api.go:37 +0x3f1
ericwj commented 4 years ago

If that is caused by an error from the go API it is still wrong.

ericwj commented 4 years ago

And I expect it to enumerate all of Y:\ and not Y:\Source which would be go API error #2 in this call.

networkimprov commented 4 years ago

Pls lets document Go API bugs in a different thread! This is about my api app!

ericwj commented 4 years ago

You're the best 🥇

ericwj commented 4 years ago

But yeah I guess iFi might be null if there is an error - not totally sure because I would expect Walk to crash before calling your code reading the docs precisely.

networkimprov commented 4 years ago

The panic you found in Walk is because the FileInfo is null, I think. I'll fix that in next rev. If Walk encounters an error, it will be reported as an error result.

Any more requests re input & output defs? My next step is to add the rest of the filepath functions...

Doesn't PS have an alternate quoting feature (e.g. here-doc) so you don't have to escape " chars? Anyway you have a PS script to generate JSON from whatever format you'd like to use on command line.

ericwj commented 4 years ago

here-doc? doesn't say much to me. There is single quote strings but although that removes the double double quotes it does obviously nothing for the required JSON escaping. \ is already just \ in PowerShell.

Well, I guess this is mostly OK. I had the idea to put it all in C# such that I can run Xunit, but maybe that is only needed for the C# that I'd write. I might have to spawn a lot of processes depending on what the tests will end up to be, but hey that's the unix way I guess.

networkimprov commented 4 years ago

https://en.wikipedia.org/wiki/Here_document

I could make the app loop on stdin if that's helpful. You'd prob only need to do Walk with a sep process.

That would let you redirect stdin from a file, and and then diff the output with expected (hardcoded) results.

ericwj commented 4 years ago

Yes, but then I would also need some kind of separator at least for Walk since the result is unbounded, also as-is the error result isn't being returned.

Glob well go puts that result in memory so I guess the idea is that Glob would have to be used with reasonably small result sets that would also be OK to put in one JSON object.

ericwj commented 4 years ago

And having followed your fine example and putting the assignment to aOut.Name inside an if, I get -222 but an error that looks suspiciously much like a Windows error message "Incorrect function" - for one the casing and punctuation is proper English, so maybe you have to do that switch statement from line 46 again:

PS> '{ "Api": "path.filepath.Walk", "Path": "\\\\?\\Volume{607932d3-78af-41c0-9786-dd0177e78a39}" }' | .\go.exe
{"Error":"GetFileInformationByHandle \\\\?\\Volume{607932d3-78af-41c0-9786-dd0177e78a39}: Incorrect function.","Errno":-222,"Result":"\\\\?\\Volume{607932d3-78af-41c0-9786-dd0177e78a39}"}
ericwj commented 4 years ago

Oh yeah and how can I make the executable not be named go.exe?

networkimprov commented 4 years ago

> go build api.go should give you api.exe. It gets "go" from the folder name with go build.

I've updated the code to loop on stdin and fix Walk. No more APIs yet.

networkimprov commented 4 years ago

And now I've added the rest of the filepath functions. Also changed api names to "filepath.*" from "path.filepath.*"

EDIT: I think you can use / in path literals everywhere except \\?; that might help readability.

ericwj commented 4 years ago

Yes, good trick.

It works quite well even from the PowerShell command line. I was worrying it might stay open but it neatly exits.

PS> '{ "Api": "filepath.Abs", "Path": "." }{ "Api": "filepath.Abs", "Path": "." }' | .\api.exe
{"Errno":0,"Result":"C:\\GoWindows\\src\\go"}
{"Errno":0,"Result":"C:\\GoWindows\\src\\go"}
PS>

and even

PS>
>> @'
>> { "Api": "filepath.Abs", "Path": "." }
>> { "Api": "filepath.Abs", "Path": "." }
>> '@ | .\api.exe
{"Errno":0,"Result":"C:\\GoWindows\\src\\go"}
{"Errno":0,"Result":"C:\\GoWindows\\src\\go"}
PS>
ericwj commented 4 years ago

Mmm, this is a bit obfuscating:

'{ "Api": "filepath.Separator"}' | .\api.exe
{"Errno":0,"Result":92}
PS> [char]92
\

Can you make that a string perhaps? And remove "filepath." from the "Api" parameter? It is just clutter at this time imho.

networkimprov commented 4 years ago

You'll probably have to validate os APIs! I think the input is clearer with the package prefix.

OK, can convert separators to strings. EDIT: done.

ericwj commented 4 years ago

Is this proper? I never get more than one element back.

Get-GoFilePath -SplitList "C:\tools\GoWindows\src\go\api.exe".Replace("\", "/") -Verbose
VERBOSE: where api.exe returned 0: '' (File Exists: False)
VERBOSE: Looking for 'api.exe' in the current working directory 'C:\tools\GoWindows\src\go' since it was not found on the path.
VERBOSE: Using 'C:\tools\GoWindows\src\go\api.exe'.
VERBOSE: In-Json: {  "Api": "filepath.SplitList",  "Path": "C:/tools/GoWindows/src/go/api.exe"}
VERBOSE: Out-Json: {"Errno":0,"Result":["C:/tools/GoWindows/src/go/api.exe"]}

Errno Result
----- ------
    0 {C:/tools/GoWindows/src/go/api.exe}
ericwj commented 4 years ago

Oh, wait, I get it.

PS> Get-GoFilePath -SplitList $env:Path -Verbose | select -ExpandProperty Result
C:\WINDOWS\system32
C:\WINDOWS
(abbreviated)
ericwj commented 4 years ago

Check this GoFilePath PowerShell script.

networkimprov commented 4 years ago

Hm, I was expecting a PS script with a collection of filepath API invocations demonstrating bugs, and PS commands illustrating correct results.

PowerShell api invokes Go stdlib functions

Should be: PowerShell Get-GoFilePath invokes Go stdlib path/filepath APIs ?

You have one instance of get-gof.

I don't understand the "Use like so..." lines. Is that for Command Shell? Why is . .\GoFilePath separate from get-gof? Why tabs? Should it describe how to start PS instead?

You don't mention starting powershell before PS>. There's no PS> prompt before Get-Help.

ericwj commented 4 years ago

I was expecting a PS script with a collection of filepath API invocations demonstrating bugs

I was working on that -- to do that from a clean machine. So I will be creating a .vhdx and a file share and map them in various ways before calling this function to verify the output.

Why is . .\GoFilePath separate?

Sorry, the readme has been typed quite hastily. I haven't gone through what is necessary to make it a PowerShell module, so I have just this one file that defines a single function. I have updated the readme a bit.

separate from get-gof? Why tabs?

There is only one function and it is called Get-GoFilePath. Tab is the autocomplete key in PowerShell on Linux, too, it must be.

networkimprov commented 4 years ago

Re the CL, you'll need to run go fmt on my code before posting. It becomes less readable, alas. But we can leave the original formatting alone and just run that before each rev is posted.

You might be asked for other style changes. I use identifier prefixes, which most of the world has not discovered. We'd have to put those in the original.

ericwj commented 4 years ago

I think Liam might have answered my question in golang-dev far enough up to the point where we have to create a PR; can you make chocolate of his code example?

I am writing a console app like yours now - I believe I can make it a single file, shipping the .NET runtime with it. It will be large. It will still be fairly easy to make a PowerShell module (out of it).

networkimprov commented 4 years ago

You think it's a coincidence that two Liam's are helping you? I make a nice cup of cocoa, will that do for "chocolate"? I think cinnamon will be better in it than C# tho.

That code example is in the go/ folder here. But I don't think including .NET in a CL is reasonable. We can probably get a builder configured for it. Maybe we should wait til 1.15 is out; they seem preoccupied with that.

ericwj commented 4 years ago

Maybe the code I write is just proof of what is possible and what is wrong with go such that it is enough to have your go app and a bunch of data in the go repo eventually.

networkimprov commented 4 years ago

Not even a "LOL" for my efforts! :'(

Hm, maybe api.exe needs tInput.Expected, and to compare that with tOutput.

ericwj commented 4 years ago

!? Your effects will be used, mine might evaporate.

api.exe needs tInput.Expected

Not really. Windows comes with PowerShell. Guaranteed, except for Nanoserver containers.

networkimprov commented 4 years ago

But simpler to do the comparison in Go, and bundle expected results with input.

Re efforts, I was referring to my joke after you separated me from golang-dev Liam, and asked for chocolate. Re-read the thread!

ericwj commented 4 years ago

Oww that is iffy of me lol, yeah ok well thanks for responding I guess 8)

I just had in my mind from the start that it was probably someone who had responded in one of the issues where I was involved, but since you don't use your name there and the others have names that I hadn't remembered yet.....