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

Jump to bottom

runtime: merge .go files produced by cgo into their counterparts #12952

Open
nodirt opened this issue Oct 16, 2015 · 13 comments
Open

runtime: merge .go files produced by cgo into their counterparts #12952

nodirt opened this issue Oct 16, 2015 · 13 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone
Unplanned

Comments

@nodirt
Copy link
Contributor

nodirt commented Oct 16, 2015

Merge

  • panic1.go -> panic.go
  • defs1_darwin.go + defs2_darwin.go -> defs_darwin.go
  • etc

Discussed in https://groups.google.com/forum/#!topic/golang-nuts/yuh2fzooEpM

I'd like to work on this.
@gopherbot
Copy link

CL https://golang.org/cl/15905 mentions this issue.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Oct 16, 2015
@gopherbot
Copy link

CL https://golang.org/cl/15951 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/15950 mentions this issue.

bradfitz pushed a commit that referenced this issue Oct 16, 2015
@nodirt @bradfitz
runtime: merge string1.go into string.go
881b0e7 
string1.go contents are appended to string.go as is

Updates #12952

Change-Id: I30083ba7fdd362d4421e964a494c76ca865bedc2
Reviewed-on: https://go-review.googlesource.com/15951
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bradfitz pushed a commit that referenced this issue Oct 16, 2015
@nodirt @bradfitz
runtime: rename print1.go -> print.go
d72d299 
It seems that it was called print1.go mistakenly: print.go was deleted
in the same commit:
https://go.googlesource.com/go/+/597b266eafe7d63e9be8da1c1b4813bd2998a11c

Updates #12952

Change-Id: I371e59d6cebc8824857df3f3ee89101147dfffc0
Reviewed-on: https://go-review.googlesource.com/15950
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
bradfitz pushed a commit that referenced this issue Oct 16, 2015
@nodirt @bradfitz
runtime: merge panic1.go into panic.go
9358f7f 
A TODO to merge is removed from panic1.go.
The rest is appended to panic.go

Updates #12952

Change-Id: Ied4382a455abc20bc2938e34d031802e6b4baf8b
Reviewed-on: https://go-review.googlesource.com/15905
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/16008 mentions this issue.

bradfitz pushed a commit that referenced this issue Oct 17, 2015
@nodirt @bradfitz
runtime: merge stack{1,2}.go -> stack.go
db2e73f 
* rename stack1.go -> stack.go
* prepend contents of stack2.go to stack.go

Updates #12952

Change-Id: I60d409af37162a5a7596c678dfebc2cea89564ff
Reviewed-on: https://go-review.googlesource.com/16008
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
bradfitz pushed a commit that referenced this issue Oct 18, 2015
@nodirt @bradfitz
runtime: merge race1.go -> race.go
2437575 
* append contents of race1.go to race.go
* delete "Implementation of the race detector API." comment
  from race1.go

Updates #12952

Change-Id: Ibdd9c4dc79a63c3bef69eade9525578063c86c1c
Reviewed-on: https://go-review.googlesource.com/16023
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/16024 mentions this issue.

bradfitz pushed a commit that referenced this issue Oct 19, 2015
@nodirt @bradfitz
runtime: merge proc1.go -> proc.go
386fa03 
from proc1.go to proc.go:
* prepend header comment explaining "Goroutine scheduler"
* insert m0 and g0 var defs after the comment
* append the rest

Updates #12952

Change-Id: I35ee9ae3287675cde0c1b6aeaca0a460393f2354
Reviewed-on: https://go-review.googlesource.com/16024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/16025 mentions this issue.

@nodirt
Copy link
Contributor Author

nodirt commented Oct 20, 2015

See comments in https://go-review.googlesource.com/#/c/16025/

On Tue, Oct 20, 2015, 1:01 AM stemar94 notifications@github.com wrote:

@nodirt https://github.com/nodirt What about runtime{1,2,}.go ?

—
Reply to this email directly or view it on GitHub
#12952 (comment).

@RLH
Copy link
Contributor

RLH commented Oct 20, 2015

In general this will cause a lot of our CLs to have conflicts and slow us
down as we approach the Nov. 1 code freeze. The upside seems minimal given
that any reasonable SDE will figure out where a function is regardless of
the file it is in.

On Tue, Oct 20, 2015 at 8:37 AM, Nodir Turakulov notifications@github.com
wrote:

See comments in https://go-review.googlesource.com/#/c/16025/

On Tue, Oct 20, 2015, 1:01 AM stemar94 notifications@github.com wrote:

@nodirt https://github.com/nodirt What about runtime{1,2,}.go ?

—
Reply to this email directly or view it on GitHub
#12952 (comment).

—
Reply to this email directly or view it on GitHub
#12952 (comment).

@nodirt
Copy link
Contributor Author

nodirt commented Oct 20, 2015

Yes, I was worried that it will interfere with your and @aclements CLs. I can postpone it.

I'm not sure how it slows you down though. https://go-review.googlesource.com/#/c/16008/ demonstrated that git-rebase handles non-invasive changes well (I don't do invasive ones)

@aclements
Copy link
Member

It depends on how git sees the merge and rename. Typically, it will rebase changes to the largest hunk of a merged file without trouble, but it will have problems following changes to the smaller hunks over. I'll try to flip through your unmerged CLs later and give my opinion on which should be postponed. I definitely want to see this change and thank you for going to this trouble!

@aclements
Copy link
Member

CL 16024 may cause problems, but I see it's already submitted, so we'll just have to see what happens. CL 16025 (runtime*.go) will definitely cause problems, so if you don't mind I'd like to hold off on that one until things have settled down a little (it looks like that one may take a little more thought, anyway). I don't think any of the other CLs will cause any problems.

@nodirt
Copy link
Contributor Author

nodirt commented Oct 20, 2015

Yes, CL 16025 would cause problems for you. That's why I abandoned it with message "Rearranging runtime_.go is more invasive and requires some planning and coordination with runtime team. I will leave them for now.
It is not the only files I will leave. There is also unaligned_.go that have different build tags."

@mdempsky
Copy link
Member

I'd like to see the os*_foo.go files collapsed into os_foo.go. Any objections to that?

Rationale: Currently it's rather unfortunate that Gerrit sorts os1_.go files together, then os2_.go files, etc. That makes it more annoying (at least IMO) to review CLs like https://golang.org/cl/16176, where it would be nicer to see the os_foo.go and os1_foo.go changes together.

As far as I can tell, CL 16176 is the only pending CL that touches the os*.go files, but I have a couple more similar cleanups in mind.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@mdempsky @nodirt @RLH @aclements @ianlancetaylor @gopherbot