os: enable symlink creation on Windows 10 #24307

fkollmann · 2018-03-08T02:26:28Z

gopherbot · 2018-03-08T02:26:53Z

This PR (HEAD: 77812a3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-08T02:27:18Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-08T09:41:15Z

Message from Tobias Klauser:

Patch Set 1:

As Brad noted in #22874 (comment), the change should include a test to verify that including the flag doesn't accidentally break older Windows versions, especially Windows XP. Depending on the test, we might need to set the flag conditionally depending on the OS version (to be detected at runtime).

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-08T10:28:35Z

Message from Elias Naur:

Patch Set 1:

Patch Set 1:

As Brad noted in #22874 (comment), the change should include a test to verify that including the flag doesn't accidentally break older Windows versions, especially Windows XP. Depending on the test, we might need to set the flag conditionally depending on the OS version (to be detected at runtime).

According to https://golang.org/doc/go1.10, Go 1.10 is the last release to support Windows XP (and Vista). Your point remains for Windows 7 of course.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-08T14:16:01Z

This PR (HEAD: f1df0d3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-08T14:20:56Z

Message from Felix Kollmann:

Patch Set 1:

I added a test case. Here are the first results:

Windows 10 developer mode enabled:
PS D:\Projekte\go-source\src> ..\bin\go.exe run ..\test\fixedbugs\issue22874.go
Windows developer mode active: true
Success: Creating symlink succeeded

Windows 10 developer mode disabled:
PS D:\Projekte\go-source\src> ..\bin\go.exe run ..\test\fixedbugs\issue22874.go
Windows developer mode active: false
Success: Creating symlink failed with expected ERROR_PRIVILEGE_NOT_HELD error

Windows 10 running elevated:
PS D:\Projekte\go-source> bin\go.exe run test\fixedbugs\issue22874.go
Windows developer mode active: false
Success: Creating symlink succeeded

I will run the tests for Windows 7 32b/64b later today.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

fkollmann · 2018-03-08T15:01:49Z

More information on the topic is provided by Git on Windows:

https://github.com/git-for-windows/git/wiki/Symbolic-Links#allowing-non-administrators-to-create-symbolic-links

gopherbot · 2018-03-08T15:13:39Z

Message from Felix Kollmann:

Patch Set 2:

@brad had the right idea. The test fails on Windows 7:

c:\go-source\src>..\bin\go.exe run ..\test\fixedbugs\issue22874.go
Windows developer mode active: false
panic: Failed to create symlink: symlink C:\Users\FKOLLM~~1\AppData\Local\Temp\issue22874.test C:\Users\FKOLLM~~1\AppData\Local\Temp\issue22874.test.link: The parameter is incorrect.�

Compared to Go v1.10:

C:\Go>bin\go.exe run c:\go-source\test\fixedbugs\issue22874.go
Windows developer mode active: false
Success: Creating symlink failed with expected ERROR_PRIVILEGE_NOT_HELD error�

I will fix this later today.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-08T15:57:43Z

Message from Felix Kollmann:

Patch Set 2:

Testing against the Windows version won't help, since it's pending on #17835

I added a retry for the parameter error, instead. (Which probably isn't any slower.)

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-08T15:59:48Z

This PR (HEAD: 5b1ae4a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-08T16:09:37Z

Message from Felix Kollmann:

Patch Set 3:

The issue is now fixed on Windows 7:

Running elevated on Windows 7:
C:\go-source>bin\go.exe run c:\go-source\test\fixedbugs\issue22874.go
Windows developer mode active: false
Success: Creating symlink succeeded�

Running as normal user on Windows 7:
c:\go-source\src>..\bin\go.exe run ..\test\fixedbugs\issue22874.go
Windows developer mode active: false
Success: Creating symlink failed with expected ERROR_PRIVILEGE_NOT_HELD error

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-09T10:31:47Z

Message from Alex Brainman:

Patch Set 3:

(6 comments)

Thank you for fixing this.

I am away from my computers until mid next week.
I apologise in advance for slow review.

Alex

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

- moved syscall changes to internal/syscall/windows - moved unit test to os package - unit test is now using direct windows regisrtry access

gopherbot · 2018-03-09T13:05:28Z

This PR (HEAD: 5e1abfb) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-09T13:08:46Z

This PR (HEAD: a4b78cf) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

googlebot · 2018-03-12T15:30:32Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

gopherbot · 2018-03-12T15:30:46Z

This PR (HEAD: 81c6e47) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

fkollmann · 2018-03-12T18:09:06Z

Empty comment for triggering CLA check.

googlebot · 2018-03-12T18:09:11Z

CLAs look good, thanks!

gopherbot · 2018-03-13T08:36:34Z

Message from Alex Brainman:

Patch Set 6:

(22 comments)

More comments. Thank you.

Alex

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-28T14:00:29Z

Message from Tobias Klauser:

Patch Set 11: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-28T14:00:56Z

Message from Gobot Gobot:

Patch Set 11:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=54be42d3

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-28T14:11:24Z

Message from Gobot Gobot:

Patch Set 11: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-04T23:03:25Z

Message from Felix Kollmann:

Patch Set 11:

Any news? Waiting for feedback or submission :) .

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-05T10:05:14Z

Message from Alex Brainman:

Patch Set 11:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-05T13:15:37Z

Message from Felix Kollmann:

Patch Set 11:

(1 comment)

Thanks for the feedback!

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-06T08:32:18Z

Message from Alex Brainman:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-06T22:28:12Z

This PR (HEAD: bab252e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-04-06T22:31:02Z

Message from Felix Kollmann:

Patch Set 12:

(1 comment)

Thanks for the feedback!

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-16T21:54:08Z

Message from Felix Kollmann:

Patch Set 12:

What's the status here? Waiting for feedback or submission :)

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T07:52:12Z

Message from Tobias Klauser:

Patch Set 12:

Patch Set 12:

What's the status here? Waiting for feedback or submission :)

Your last reply on Alex' review comment (PS3, Line 383) was

I will change the code:

but there was no new patch set uploaded. Maybe you forgot to push your latest changes?

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T08:29:57Z

Message from Alex Brainman:

Patch Set 12:

What's the status here? Waiting for feedback or submission :)

What Tobias said. I think it is your turn.

Alex

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T08:59:38Z

Message from Felix Kollmann:

Patch Set 12:

Sry for the confusion. I usually submitted the reply after uploading the new patch. I see why this is confusing and will improve on the workflow. Thanks for the feedback!

The announced change is already part of patch set 12.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T09:01:51Z

Message from Tobias Klauser:

Patch Set 12: Run-TryBot+1

Patch Set 12:

Sry for the confusion. I usually submitted the reply after uploading the new patch. I see why this is confusing and will improve on the workflow. Thanks for the feedback!

The announced change is already part of patch set 12.

Sorry, I didn't check careful enough then. Gerrit can be a bit confusing at times :)

I'll let Alex decide on the actual submission. Thanks.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T09:01:55Z

Message from Gobot Gobot:

Patch Set 12:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=56222dc7

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T09:09:23Z

Message from Alex Brainman:

Patch Set 12:

The announced change is already part of patch set 12.

I can see that now. I will review your change when I have time.

Thank you.

Alex

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-17T09:11:37Z

Message from Gobot Gobot:

Patch Set 12: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-18T09:41:44Z

Message from Alex Brainman:

Patch Set 12:

(3 comments)

Nearly looking good.

Thank you.

Alex

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-18T21:04:56Z

This PR (HEAD: 3ba7abc) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-04-18T21:06:18Z

Message from Felix Kollmann:

Patch Set 13:

(3 comments)

Thanks for the feedback! I uploaded a new patch.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-19T09:58:44Z

Message from Alex Brainman:

Patch Set 13: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-19T09:58:53Z

Message from Gobot Gobot:

Patch Set 13:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=1cd37e35

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-19T10:08:54Z

Message from Gobot Gobot:

Patch Set 13: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99337.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-04-19T10:10:53Z

This PR is being closed because golang.org/cl/99337 has been merged.

Fixes #22874 Change-Id: Ia30fc8df39e88fbc2939a4490c34da8dd5815a94 GitHub-Last-Rev: 3ba7abc GitHub-Pull-Request: #24307 Reviewed-on: https://go-review.googlesource.com/99337 TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alex Brainman <[email protected]>

os/file: enable symlink creation o Windows 10

77812a3

fixes #22874

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Mar 8, 2018

test: added test for fix #22874

f1df0d3

os/file: fixed compatibility with older Windows versions

5b1ae4a

os: improved code based on review feedback

5e1abfb

- moved syscall changes to internal/syscall/windows - moved unit test to os package - unit test is now using direct windows regisrtry access

os: fixed 'file' unit test logging on windows

a4b78cf

os: fixed typo in Windows version number

81c6e47

googlebot added cla: no Used by googlebot to label PRs as having an invalid CLA. The text of this label should not change. and removed cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. labels Mar 12, 2018

googlebot added cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. and removed cla: no Used by googlebot to label PRs as having an invalid CLA. The text of this label should not change. labels Mar 12, 2018

djdv mentioned this pull request Apr 6, 2018

Windows initiative 2018 ipfs/kubo#4808

Closed

9 tasks

os: improved code based on review feedback

bab252e

fkollmann added 2 commits April 18, 2018 23:01

Merge remote-tracking branch 'origin/master'

4d68b14

os: improved code based on review feedback

3ba7abc

gopherbot closed this Apr 19, 2018

os: enable symlink creation on Windows 10 #24307

os: enable symlink creation on Windows 10 #24307

Conversation

fkollmann commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

fkollmann commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 8, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

googlebot commented Mar 12, 2018

gopherbot commented Mar 12, 2018

fkollmann commented Mar 12, 2018

googlebot commented Mar 12, 2018

gopherbot commented Mar 13, 2018

gopherbot commented Mar 28, 2018

gopherbot commented Mar 28, 2018

gopherbot commented Mar 28, 2018

gopherbot commented Apr 4, 2018

gopherbot commented Apr 5, 2018

gopherbot commented Apr 5, 2018

gopherbot commented Apr 6, 2018

gopherbot commented Apr 6, 2018

gopherbot commented Apr 6, 2018

gopherbot commented Apr 16, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 17, 2018

gopherbot commented Apr 18, 2018

gopherbot commented Apr 18, 2018

gopherbot commented Apr 18, 2018

gopherbot commented Apr 19, 2018

gopherbot commented Apr 19, 2018

gopherbot commented Apr 19, 2018

gopherbot commented Apr 19, 2018