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

reporting problems to Go project #2

Open
networkimprov opened this issue Jul 9, 2020 · 52 comments
Open

reporting problems to Go project #2

networkimprov opened this issue Jul 9, 2020 · 52 comments

Comments

@networkimprov
Copy link
Collaborator

networkimprov commented Jul 9, 2020

When you're ready to report the problems you've found to the Go project, I suggest you:

a) open a new Go issue (and close the "EvalSymlinks..." issue)
b) reference the CL you create in the issue attach the files needed to reproduce the bugs
c) include the expected and actual output in the issue text, as a code block
d) avoid any indication of grumpiness (even if justified :-)

Don't point to this repo, or mention me. The Windows maintainer bears a grudge because of golang/go#32088 :-/

@networkimprov
Copy link
Collaborator Author

networkimprov commented Jul 9, 2020

@ericwj pls ack when you have a chance. Thanks :-)

@ericwj
Copy link
Owner

ericwj commented Jul 9, 2020

Shouldn't I wait for a response from golang-dev? Also heh if they were grumpy about that how will they feel about me by now?

Alas you mentioned this issue by including that link - this issue is now part of that go issues history.

@networkimprov
Copy link
Collaborator Author

Yes, wait for a golang-dev response.

No one will see that link on a closed issue; I considered that. You haven't had a months-long argument with an ornery Go maintainer :-)

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

I hope I will see the response. I have had my gmail quarantained for decades by now.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

Well ok plural is exaggerated. Less than 2.

@networkimprov
Copy link
Collaborator Author

networkimprov commented Jul 10, 2020

It's up: https://groups.google.com/forum/#!topic/golang-dev/kbslpK1VGo8

It looks good to me; but use "CL" not "PR". Code review is managed on Gerrit in "change lists". Submission via Github PR is for convenience.

For example, this is about a path bug in os.RemoveAll() which we should also test!

https://go-review.googlesource.com/c/go/+/214437/
EDIT: associated issue is golang/go#36375

EDIT2: bug re os.MkdirAll() golang/go#39785

@networkimprov
Copy link
Collaborator Author

There isn't much value in debating the Windows maintainer. Just complete your test battery.

Note that anyone can post a CL, but to have it committed to the repo, it has to include fixes for any bugs it demonstrates. As there aren't many Go contributors working on Windows, it may have to be you. I'll continue to support you if so.

I have little experience with CL tests, but you may need to invoke the PS script from a Go test file (which the builder runs automatically), see https://golang.org/pkg/os/exec/#Command. I'll make an example in this repo.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

I might follow your advice, clean up the original issue text such that it contains basically the general idea of my last post there and change the title (back) to "no-op EvalSymLinks on Windows", what do you say?

@networkimprov
Copy link
Collaborator Author

I wouldn't invest any more time in that issue; its discussion is long and meandering. We need a new one with a log of correct vs actual filepath.* results, and a discussion solely about that.

Misuse of EvalSymlinks by Windows apps is not Go's fault. We can fix stdlib bugs, but its expected behavior can't change.

I've pushed code to call a script from go test which the builders run every time; see the readme. I think your CL will need that.

You may need to ping your golang-dev post next week; I think Google folks took the week off for July 4th.

@networkimprov
Copy link
Collaborator Author

After we fix the filepath bugs, you could try a proposal for a new API

func EvalSymlinksFrom(path string, evaluationRoot string) (string, error)

But you'd have to document several important use cases to justify it. The Go team is reluctant to add APIs.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

We might wait a week for golang-dev if an answer ever comes, looking at how long we've waited and the number of responses to questions that were posted after mine.

@networkimprov
Copy link
Collaborator Author

I've found that list to be helpful and responsive.

I guess you're having a hard day. You're alienating the Go team in the original issue after I suggested you leave it alone, and reducing our chances of getting some bugs fixed. You should apologize and work on something else for a day or two.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

What did you consider grumpy? I just outlined the reving strategy of some other big projects like go.

Maybe I shouldn't have said I consider their reviewing not serious, but yeah well I guess it doesn't matter how obvious that is from that 'VolumeName' issue it might still not be nice to hear. To me it isn't personal. It's more what the world would consider preferable and that they can evolve if they choose to listen to criticism like that.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

I was much more harsh in every mention of "Google" before these last few posts and I think I was spot on in both my assessment of Google and their stance towards Windows.

@ericwj
Copy link
Owner

ericwj commented Jul 10, 2020

Again not personal, but where the Microsoft jargon talks about 'the pit of succcess' where people should fall in by doing standard things the default way or doing nothing (special), Google is squarly saying 'your bugs aren't ours' where imho they are.

@networkimprov
Copy link
Collaborator Author

BTW, you'll see feedback for your previous CL here
https://go-review.googlesource.com/c/go/+/241278

Presumably the Windows maintainer will accept it, and it will land in Go 1.16.

@ericwj
Copy link
Owner

ericwj commented Jul 11, 2020

I'm impressed that you can deduce that. Oh wait, there's some +2's yeah. OK good, that solves the most important issue. I'm guessing you know by heart that it is too late for 1.15.

@ericwj
Copy link
Owner

ericwj commented Jul 11, 2020

Should I change the docs through modifying the comments above func EvalSymLinks in path.go?

@networkimprov
Copy link
Collaborator Author

Yes, docs live in the comments above each public function.

@ericwj
Copy link
Owner

ericwj commented Jul 11, 2020

// EvalSymlinks returns the path name after the evaluation of any symbolic
// links.
// If path is relative the result will be relative to the current directory,
// unless one of the components is an absolute symbolic link.
// EvalSymlinks calls Clean on the result.
// On Windows, use of this function is unsuitable for most applications
// unless they create, manage and delete links on behalf of the user
// and know for certain that links managed by the application are the
// only links in the path(s) to links they manage.
func EvalSymlinks(path string) (string, error) {
	return evalSymlinks(path)
}

@ericwj
Copy link
Owner

ericwj commented Jul 12, 2020

I am confused by git regularly, including having to guess what application commands do in VS/VSCode, and having GitHub involved is adding to that.

Can you follow this step-by-step before I make another error?

I have created branch ericwj-evilsymlinks in ericwj/go with a PR, the problem probably being (I think) that it is merged into master. I use VS Code and git status tells me On branch ericwj-evilsymlinks (...) up-to-date (...) clean and I am tempted to use VS Codes Undo Last Commit and then commit that to ericwj/go and merging that with master too.

Correct?

@networkimprov
Copy link
Collaborator Author

Most everyone is regularly confused by git.

Undo-last-commit may not revert (it may "reset"). I think you should revert and merge the revert commits to master. I'm not sure that Undo-last would be mirrored correctly on Gerrit.

Yes, merging the new branch to master was the problem. In general, PRs should not use the master branch.

@ericwj
Copy link
Owner

ericwj commented Jul 12, 2020

How do I now get my branch to become a new PR? I thought that would happen the first time. Plus git revert actually reverted the change on that branch, but I can just copy/paste once more.

And how do I update my fork? It is behind golang/go quite a bit now

@networkimprov
Copy link
Collaborator Author

Don't worry about the PR til you have agreement on the issue.

And for heaven's sake, remove your angry comments, and revise the issue text to reflect the fact that some apps have good reason to use this API. You don't know of every situation where this is called; stop pretending to.

And stop and think before you write any text or code. You're producing too much of both, and damaging your productivity -- and that's making you even more unhappy.

@ericwj
Copy link
Owner

ericwj commented Jul 12, 2020

I changed it, deleted some. Decoupled the word 'problem' from EvalSymLinks, making clear that I meant 'system configuration problem (disk full, disaster recovery, etc) under the responsibility of the administrator'.

I am not always fully aware how it comes across, do I look fuzzy warm and happy now or is it still too lengthy and grumpy?

@networkimprov
Copy link
Collaborator Author

It's better, but I'd delete the comments, and add the MS link to the issue text along with its quote "Neither users nor applications need ..."

@networkimprov
Copy link
Collaborator Author

Actually the first paragraph in "EvalSymLinks cannot be used to identify volumes ..." is a bit garbled. Would help to simplify that description.

@networkimprov
Copy link
Collaborator Author

I think you should stop editing it now; it's not a book manuscript :-)

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

I am happy though :)

@networkimprov
Copy link
Collaborator Author

Great! Quit while you're ahead.

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

I will, thanks Liam.

@networkimprov
Copy link
Collaborator Author

Don't admit ignorance unless you have to. It's enough to say "go vet could improve on this solution"

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

Well, I have been documenting my lifetime go achievement en plain publique as a principal point more or less proudly several times as well. Ignorance about go by choice.

Although that was part of proofing that I was justified being angry and frustrated even to be present in golang/go and to slap people for writing sloppy, rookie code, not testing it properly if at all, inventing random errors, reviewing like a wet newspaper and leaving unsuspecting regular end users across the world hanging to dry doing so.

@networkimprov
Copy link
Collaborator Author

Um, how long have you been writing software? You've never seen such things before? :-)

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

Not in this kind of project, no. I mean seeing its inventors, backers, its profile, and its arrogance comparing itself with C/C++, I would expect at least the same thorough, serious, rigorous review process as with .NET. Not that they don't make errors, but they can fix some in reasonably short time. Imho go will be obsoleting itself fairly quickly by locking itself into the issues that plague C/C++. They must be aiming to be in the top of StackOverflow most hated list faster than C/C++ by being unable to change anything already in version 1.

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

Dreaded, they call it most dreaded.

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

how long have you been writing software

I have been a Microsoft guy from 2000 at least, Delphi before that, Borland Pascal and ha gwbasic is where I started, age 12 or so. Guess I have been lucky to not encounter these issues too often.

I guess is why I used to have antipathy for Linux and open source software ported from it. Still today, I thoroughly dislike working on Linux servers, it is a tremendously time consuming, inflexible and error prone environment imho. Software like Gimp, Inkscape, FontForge and others I've used but it's never painless, looks horrible, filled with bugs and issues, yes, like buttons that are 0.1mm square on my system. I started believing in open source when Microsoft went all in and showed how things could be.

The antonym for 'loved' is 'hated'. It's useful software sometimes, and free, so I shouldn't complain too loud, but porting to Windows is indeed very often not pretty.

@networkimprov
Copy link
Collaborator Author

Go mindshare is low & flat. The team knows it's not the world beater they aspired to. Perhaps partly owing to their resistance to adding simple, oft-requested features. That's compounded by weak community-building, and amateur marketing. That said, there aren't many options in cross-platform, compiled languages.

But Google is well known for both arrogance, and poor performance in many of its efforts. The culture is famously maintenance-averse.

https://trends.google.com/trends/explore?date=today%205-y&q=%2Fm%2F07sbkfb,%2Fm%2F05z1_,%2Fm%2F07657k,%2Fm%2F0bbxf89,%2Fm%2F09gbxjr

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

What is this? Search history? There might be heavy bias; I for one literally use Google search about once every quartal. It is eternally regrettable Microsoft did not succeed in mobile. I still have a Windows phone, without Whatsapp since start of the year when Facebook hit the kill switch. I am very displeased I will eventually have to choose between the anti-Microsoft-trust members Tim Crook and Godzilla.

@ericwj
Copy link
Owner

ericwj commented Jul 13, 2020

options in cross-platform, compiled languages

I hear very good stories about rust. It crossed 1% volume a short while ago, who knows it will be growing exponentially. I can do anything I want with the combo C++/C# on all operating systems I might ever encounter in the wild - with excellent tooling including remote debugging at least with gdb and cmake/clang/llvm/gnu support if required. Also locally on my Windows box to a WSL2 Linux kernel, I might be building a whole series of apps on Alpine soon although that won't include much C++ if any at all.

@networkimprov
Copy link
Collaborator Author

Google Trends is data from searches, yes. What's interesting about the graph is the line slopes, not their relative positions, altho I believe both are relevant.

Rust has terrible compile times and a steep learning curve, but excellent Windows support. Nobody really mentions C# as a cross-platform environment, I guess because the .NET library isn't widely used on Linux, MacOS, Android, iOS... What's Alpine?

This is my project, if you haven't looked: https://github.com/networkimprov/mnm-hammer/blob/master/README.md

@ericwj
Copy link
Owner

ericwj commented Jul 14, 2020

Ah, 5-6-5. I pretty much have to delete everything to see any reliable trend though. Go appears to be flattening, yeah - wouldn't conclude that it will start declining yet.

I'm surprised to see C# decline steadily. Everything I hear points in the opposite direction since .NET Core is here, actually especially for containers/microservices/grpc on Linux. You probably don't hear about it because if you use go you're in another correlation cloud - usually similar as people around you. It's easy to start from dot.net on most systems, either through the package manager (winget) or through unzipping a download.

C#/ASP.NET Core is pretty blazing fast on TechEmpower, certainly faster than every other, similarly practical and productive framework. Microsoft is absolutely dead serious about performance. .NET 5 coming November will have many new improvements.

Alpine Linux can run on a VM of less than 128MB, diskless if you want, or a container of less than 5MB. Although well I just write whatever I need. I can pretty much switch mostly effortless for this kind of projects.

Mnm yeah I have seen it from your profile, I was wondering what it was but hadn't looked into it more than glancing. Is that what you do in your free time? You didn't test chromium Edge? Sound good too? :)

@ericwj
Copy link
Owner

ericwj commented Jul 14, 2020

I presumed we'll just have to wait for them to review and give their opinion for 39786 and to see how far they're willing to go on 40180, right? That could be weeks. Kind of weird that it is absolutely quiet given what happened before.

@ericwj
Copy link
Owner

ericwj commented Jul 15, 2020

Does this fit expectations?

PS C:\tools\GoWindows\test\goPathFilePath> Get-Content .\os.tsv.txt
Traits  Api     Arg1    Arg2    Value1  Value2
None    Chdir   Path            C:\Users
None    Chdir   Path            C:
PS C:\tools\GoWindows\test\goPathFilePath> .\makejson.ps1 -InFile .\os.tsv.txt -OS -TestVolumeGuidPath = $part.Guid -Verbose | ..\..\src\go\api.exe
VERBOSE: RemainingArguments: -OS, -TestVolumeGuidPath, =, {b4274801-f0ea-4f0a-acd9-d7bf7869d3b6}
VERBOSE: Splat: OS=True, TestVolumeGuidPath={b4274801-f0ea-4f0a-acd9-d7bf7869d3b6}
VERBOSE: #0000 Set: os Input: @{Traits=None; Api=Chdir; Arg1=Path; Arg2=; Value1=C:\Users; Value2=}
VERBOSE: {"Path":"C:\\Users","Api":"os.Chdir"}
VERBOSE: #0001 Set: os Input: @{Traits=None; Api=Chdir; Arg1=Path; Arg2=; Value1=C:; Value2=}
VERBOSE: {"Path":"C:","Api":"os.Chdir"}
VERBOSE: #0002 Set: os Input: @{Traits=None; Api=Chdir; Arg1=Path; Arg2=; Value1=C:\; Value2=}
VERBOSE: {"Path":"C:\\","Api":"os.Chdir"}
VERBOSE: #0003 Set: os Input: @{Traits=None; Api=Chdir; Arg1=Path; Arg2=; Value1=C:\\; Value2=}
{"Errno":0,"Result":null}
{"Errno":0,"Result":null}
{"Errno":0,"Result":null}

The results aren't coming line-by-line so it looks out of order but this can be chained to write to a file which will only contain JSON results or yet more chaining to write tab-separated too. That opens beautifully in Excel, or can be imported beautifully, depending on region and language settings in my experience. So both writing tests and reviewing results can be done in Excel if required. Tabs don't show well on GitHub, though.

@ericwj
Copy link
Owner

ericwj commented Jul 15, 2020

It'll probably take only a few minutes given two JSON files to format it as a git diff with expected and actual values, or even to pretty it up showing only what is necessary like so

+ os.Chdir C:\
- os.Chdir \\?\C: #0!=87 ''!='chdir \\?\C:: The parameter is incorrect.'
+ os.Chdir \\?\C:\.. #123 'chdir \\?\C:\..: The filename, directory name, or volume label syntax is incorrect.'

Although I would have to regex compare the error messages, make them exactly equal or compare just error numbers.

@networkimprov
Copy link
Collaborator Author

To my eye, it's overengineering to make a script that generates the JSON from a tab-delimited file. I'm worried you're going to burn out before assembling very many actual tests!

JSON can be formatted in a text file however you like, with newlines and indents. It's perfectly readable that way. The input format could be changed from objects to arrays, eliminating field names.

Also, I think we were discussing this issue on the other thread, where I had mentioned inserting local ids into strings from command-line arguments.

@ericwj
Copy link
Owner

ericwj commented Jul 16, 2020

Get-Content $path | ConvertFrom-Csv -Delimiter "``t" | ConvertTo-Json -Compress

@ericwj
Copy link
Owner

ericwj commented Jul 16, 2020

Yes, I'm converting {test} to (Get-Vhd test.vhdx | Get-Disk | Get-Partition).Guid

So I write tests like this

Traits	Api	Arg1	Arg2	Value1	Value2
VolId	Chdir	Path		\\?\Volume{test}\NonExisting\..	
VolId	Getwd			

@networkimprov
Copy link
Collaborator Author

Sorry, that was a lil grumpy... can we move topic back to #3?

@networkimprov
Copy link
Collaborator Author

Re go/39786, now you see what we're dealing with :-/

@ericwj
Copy link
Owner

ericwj commented Jul 22, 2020

lol

@ericwj
Copy link
Owner

ericwj commented Jul 22, 2020

See now this is where Google comes in to make it so that these hardships don't fall on a project like go.

Microsoft has been pulling hard to get all sorts of operating systems working full featured on Hyper-V, submitting PR's to the Linux kernel, because they had to run in Azure, obviously, but then the next step to get .NET tested on these OS's is a breeze.

But well yeah I don't hear Google say

Google ♥ Windows

Hell freezes over as Microsoft joins the Linux Foundation
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants