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.

Sign up for GitHub

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

Jump to bottom

syscall: add S_IRWXG and S_IRWXO to FreeBSD types #26675

Closed
wants to merge 1 commit into from
Closed

syscall: add S_IRWXG and S_IRWXO to FreeBSD types #26675

wants to merge 1 commit into from

Conversation

janl
Copy link
Contributor

@janl janl commented Jul 29, 2018

Companion PR to golang/sys#13

@googlebot 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 Jul 29, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 3: New patch set was added with same tree, parent, and commit message as Patch Set 2.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 3:

Did you modify the ztypes file by hand or did you run the generator?

If there was unexpected noise, you were likely using the wrong FreeBSD version.

Also, we need to update freebsd/arm and freebsd/386.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 28528:

Patch Set 3:

Patch Set 3:

Did you modify the ztypes file by hand or did you run the generator?

I used the generator, but left out the unrelated changes. These constants are rather old as in BSD 4.4 old, so I’m not too worried about value variance.

If there was unexpected noise, you were likely using the wrong FreeBSD version.

What is the right FreeBSD version? That was FreeBSD 11.2-RELEASE #0 which is fairly recent.

Also, we need to update freebsd/arm and freebsd/386.

I don’t have either of these to run the generator on, but I’d be happy to apply the same changes if someone confirms that it’s the right thing to do. If not, I‘d consider this out of scope for this PR.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 3:

Actually I'm not sure which FreeBSD version we've been using. Maybe the git history has details in a commit message for one of those FreeBSD files?

Tobias, do you know?

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 3:

Patch Set 3:

Patch Set 3:

Did you modify the ztypes file by hand or did you run the generator?

I used the generator, but left out the unrelated changes. These constants are rather old as in BSD 4.4 old, so I’m not too worried about value variance.

If there was unexpected noise, you were likely using the wrong FreeBSD version.

What is the right FreeBSD version? That was FreeBSD 11.2-RELEASE #0 which is fairly recent.

Also, we need to update freebsd/arm and freebsd/386.

I don’t have either of these to run the generator on, but I’d be happy to apply the same changes if someone confirms that it’s the right thing to do. If not, I‘d consider this out of scope for this PR.

You should be able to run at least parts of the generator for freebsd/386 on freebsd/amd64. But these constants are the same for 386, arm and amd64, so I think it should be safe to manually apply the same changes for 386 and arm if you don't have those.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 3:

Patch Set 3:

Actually I'm not sure which FreeBSD version we've been using. Maybe the git history has details in a commit message for one of those FreeBSD files?

Tobias, do you know?

I think the current state is a mixture of different FreeBSD versions. When regenerating I usually leave out all non-related changes, i.e. constants removed/added in newer/older versions. AFIAR I used 10.4 for all my FreeBSD changes.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 5: New patch set was added with same tree, parent, and commit message as Patch Set 4.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 28528:

Patch Set 5:

latest push includes updates for arm & 386

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 5: Run-TryBot+1 Code-Review+2

Thanks.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=5347e01c

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 5:

Thanks

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 5:

Sorry, I somehow mixed the CLs for syscall and x/sys. Both LGTM but I guess the one for syscall has to wait until after the Go 1.11 release, thus added the "wait-release" hashtag.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 5: -Code-Review

(1 comment)

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 5: TryBot-Result+1

TryBots are happy.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 28528:

Patch Set 5:

Patch Set 5: -Code-Review

(1 comment)

latest push fixes this. It’s on GitHub already, Gerrit sure catches up soon.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 6: Commit message was updated.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 7: New patch set was added with same tree, parent, and commit message as Patch Set 6.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 8: New patch set was added with same tree, parent, and commit message as Patch Set 7.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 9: New patch set was added with same tree, parent, and commit message as Patch Set 8.

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

@janl janl changed the title unix: add S_IRWXG and S_IRWXO to FreeBSD types syscall: add S_IRWXG and S_IRWXO to FreeBSD types Aug 8, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 9:

Patch Set 5:

Patch Set 5: -Code-Review

(1 comment)

latest push fixes this. It’s on GitHub already, Gerrit sure catches up soon.

Reading https://golang.org/wiki/GerritBot#how-does-gerritbot-determine-the-final-commit-message, I think you'll also need to change the PR title/description because GerritBot uses those for the commit message in Gerrit.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 10: Commit message was updated.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 28528:

Patch Set 9:

Patch Set 9:

Patch Set 5:

Patch Set 5: -Code-Review

(1 comment)

latest push fixes this. It’s on GitHub already, Gerrit sure catches up soon.

Reading https://golang.org/wiki/GerritBot#how-does-gerritbot-determine-the-final-commit-message, I think you'll also need to change the PR title/description because GerritBot uses those for the commit message in Gerrit.

Thanks for the pointer! I fixed the PR title on GitHub, but Gerrit won’t let me because I’m signed up here with a different email address because $reasons. If Gerrit doesn’t sync this over, would you be able to overwrite this? I won’t be able to get my committer email set up with Gerrit. Sorry for the trouble!

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 11: New patch set was added with same tree, parent, and commit message as Patch Set 10.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 19560:

Patch Set 11:

Patch Set 9:

Patch Set 9:

Patch Set 5:

Patch Set 5: -Code-Review

(1 comment)

latest push fixes this. It’s on GitHub already, Gerrit sure catches up soon.

Reading https://golang.org/wiki/GerritBot#how-does-gerritbot-determine-the-final-commit-message, I think you'll also need to change the PR title/description because GerritBot uses those for the commit message in Gerrit.

Thanks for the pointer! I fixed the PR title on GitHub, but Gerrit won’t let me because I’m signed up here with a different email address because $reasons. If Gerrit doesn’t sync this over, would you be able to overwrite this? I won’t be able to get my committer email set up with Gerrit. Sorry for the trouble!

Seems to have been enough to change it on Github, now it's correct in Gerrit as well. Thanks.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 12: New patch set was added with same tree, parent, and commit message as Patch Set 11.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 13: New patch set was added with same tree, parent, and commit message as Patch Set 12.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 14: New patch set was added with same tree, parent, and commit message as Patch Set 13.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 15: New patch set was added with same tree, parent, and commit message as Patch Set 14.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 16: New patch set was added with same tree, parent, and commit message as Patch Set 15.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 17: New patch set was added with same tree, parent, and commit message as Patch Set 16.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 17:

What about OpenBSD and DragonFly?

I think we should probably just say that the answer here is to use x/sys/unix.

Tobias, opinion?

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5206:

Patch Set 17:

What about OpenBSD and DragonFly?

I think we should probably just say that the answer here is to use x/sys/unix.

Tobias, opinion?

I'm inclined to think that since the constants are defined in the syscall package on Darwin, GNU/Linux, and NetBSD that it's OK to define them in the syscall package on other systems as well. That will promote portability for packages that currently use syscall, even if it would be better in the long run for them to move to x/sys/unix.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 17: Code-Review+2

Patch Set 17:

What about OpenBSD and DragonFly?

I think we should probably just say that the answer here is to use x/sys/unix.

Tobias, opinion?

I'm inclined to think that since the constants are defined in the syscall package on Darwin, GNU/Linux, and NetBSD that it's OK to define them in the syscall package on other systems as well. That will promote portability for packages that currently use syscall, even if it would be better in the long run for them to move to x/sys/unix.

Okay, SGTM. Tobias, feel free to send NetBSD and DragonFly equivalent CLs sometime if you'd like.

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 18: Patch Set 17 was rebased

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 18: Run-TryBot+1

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 18:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=33f6ec89

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

@gopherbot
Copy link
Contributor

Message from Gerrit User 5976:

Patch Set 18: TryBot-Result+1

TryBots are happy.

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

@gopherbot
Copy link
Contributor

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

@gopherbot gopherbot closed this Aug 21, 2018
gopherbot pushed a commit that referenced this pull request Aug 21, 2018
@janl @bradfitz
syscall: add S_IRWXG and S_IRWXO to FreeBSD types
0ca3203 
Companion PR to golang/sys#13

Change-Id: I097fc97912840eb69ca232eded6ba939de0fead9
GitHub-Last-Rev: f8a8f7d
GitHub-Pull-Request: #26675
Reviewed-on: https://go-review.googlesource.com/126621
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@janl @gopherbot @googlebot