Core devs: What is the status of my external linking and shared library CLs for Go 1.2?
Elias Naur
Hi,
The Go 1.2 freeze date is still a while off, but close enough that I’m starting to wonder how many, if any, of my pending CLs will be included.
Linux/arm external linking
Support for external linking on linux/arm is implemented with the two CLs 10499043 and 10271047. Both are a month old and have not yet received a review, and I’m not sure why. External linking is already agreed upon (issue 4069), have been implemented for several other platforms in Go 1.1, I was encouraged to work on linux/arm (issue 5590), and finally it will be useful for several future features like shared libraries, Android support and, AFAIU, it will be helpful for the darwin/arm port.
Therefore, I only see lack of (review) resources as reason to not include linux/arm external linking in Go 1.2. That would be an unfortunate outcome, so I would like an update on the status of the review process, if there’s any hold ups I have overlooked and so on.
Go shared library support
Go shared library support is implemented for linux/amd64 and linux/arm through the CLs 9733044 and 9738047. Shared library support as a feature has been accepted (issue 256, partly issues 4848 and 2790), but the exact semantics have not been decided yet. I started a discussion at
where I believe I have addressed all the issues that popped up in response to my proposal. However, the discussion died almost a month ago, while the CLs themselves were created almost two months ago.
Not including shared library support in Go 1.2 would be unfortunate, but I can understand if there’s not enough resources to think the design through. However, do note that issue 256 is from 2009 and that my initial attempt at shared library support is almost a year old (fall 2012).
Failing a final design decision and review of my CLs, here’s my suggestions for alternatives for Go 1.2 listed from most to least desirable (to me):
1. Include the tooling CL (9733044) and skip the runtime parts for now (CL 9738047).
I’m aware of the hesitation to include parts of a feature not thoroughly discussed, but I argue that
- The tooling changes are low level enough to be necessary for however the semantics of go shared libraries end up.
- I already had support for the -shared flag included in the tools from the initial attempt at shared libraries I developed last fall. External linking was not impemented at that time, so the tooling CL is less an addition and more a change to use the host linker for the heavy lifting instead.
2. The same as 1. but instead of the full tooling support, apply only the changes from the tooling CL that remove the parts of the existing -shared flag support that can be replaced by external linking. This will ease the burden of maintaining the shared library changes externally, and the removed code is almost guaranteed not to be needed for the eventual shared library code.
3. Some kind of commitment to include shared library support in Go 1.3. With the discussion dead, I would appreciate some kind of indication of how long I have to maintain the CLs unofficially and when to expect progress.
External contributers
I often see people on the mailing lists mention that Go is a true open source project with many external contributors and that it is not simply a Google project with a public source code repository. I also see them encourage external contributors if they have an itch to scratch. However, I can’t help being slightly put off by my experience so far with contributing, mainly because my CLs take a long time to be reviewed. I do see CLs submitted from external contributors that haven’t been part of the project for years, but they’re mostly small or trivial.
I won’t speak for others, but I would appreciate it tremendously if something was done to lower the lag between hg mail and the review. I have no problem waiting days or a few weeks, but I don’t think a month and more for CLs that follow agreed design is reasonable, even for larger changes. Of course, my CL sample size is small, so I apologize if the delays are caused by special circumstances I’m unaware of.
Finally, I’d like to suggest the codereview plugin be expanded to support patch queues or alternatively to work with the hq mq extension. I can see why the core devs don’t need mq because their CLs are applied soon after creation, but for me, hq mq was essential to manage up to 6 levels of separate but dependent patches. Converting and updating mq patches to CLs is an error-prone and time consuming process.
I would file an issue for this, but I noticed that mq was indeed supported at some time but was since removed. So I’m unsure whether patch queue support is acceptable in the codereview plugin at all.
- elias
mortdeus
Elias Naur
Jan Mercl
> The last time I tried that, I didn't have permission to post in golang-dev.
> In any case, the core devs are quite active in here, and according to the
> contribution guidelines design discussions should be done on golang-nuts.
golang-nuts. Code review discussions _after sending a CL_ are normally
held at golang-dev (it's a "standard" CC of any Rietveld message).
AFAIK, access to golang-dev is granted manually on first post to it
(to prevent spammers), but I believe _every_ non spam mail address is
then allowed to send messages to golang-dev.
I have no idea why you cannot post at golang-dev, but this explains
why, for example CL 10271047 [0] gets ignored: It's only reviewer is
the very list which drops your messages! (Including any "ping"
messages.)
I suggest to add at least one reviewer from the Go team to every CL
you send right from the beginning. The best person would be the one
which you've discussed with the CL before sending it, I believe.
FYI: You can add new reviewers to CL at any time, ie. also now.
CC: golang-dev enable access request.
[0]: https://codereview.appspot.com/10271047/
-j
Elias Naur
Elias Naur
Rémy Oudompheng
> didn't see my pings from the CLs, though, is that because "golang-dev1" is
> listed as reviewer, not "golang-dev"? I followed your advice and added a
> (hopefully relevant) reviewer to the external linking CLs.
>
https://codereview.appspot.com/user/golang-dev1
Russ Cox
Linux/arm external linking
Support for external linking on linux/arm is implemented with the two CLs 10499043 and 10271047. Both are a month old and have not yet received a review, and I’m not sure why. External linking is already agreed upon (issue 4069), have been implemented for several other platforms in Go 1.1, I was encouraged to work on linux/arm (issue 5590), and finally it will be useful for several future features like shared libraries, Android support and, AFAIU, it will be helpful for the darwin/arm port.
Therefore, I only see lack of (review) resources as reason to not include linux/arm external linking in Go 1.2. That would be an unfortunate outcome, so I would like an update on the status of the review process, if there’s any hold ups I have overlooked and so on.
Go shared library support
Go shared library support is implemented for linux/amd64 and linux/arm through the CLs 9733044 and 9738047. Shared library support as a feature has been accepted (issue 256, partly issues 4848 and 2790), but the exact semantics have not been decided yet. I started a discussion at
where I believe I have addressed all the issues that popped up in response to my proposal. However, the discussion died almost a month ago, while the CLs themselves were created almost two months ago.
Not including shared library support in Go 1.2 would be unfortunate, but I can understand if there’s not enough resources to think the design through. However, do note that issue 256 is from 2009 and that my initial attempt at shared library support is almost a year old (fall 2012).
Failing a final design decision and review of my CLs, here’s my suggestions for alternatives for Go 1.2 listed from most to least desirable (to me):
1. Include the tooling CL (9733044) and skip the runtime parts for now (CL 9738047).
I’m aware of the hesitation to include parts of a feature not thoroughly discussed, but I argue that
- The tooling changes are low level enough to be necessary for however the semantics of go shared libraries end up.
- I already had support for the -shared flag included in the tools from the initial attempt at shared libraries I developed last fall. External linking was not impemented at that time, so the tooling CL is less an addition and more a change to use the host linker for the heavy lifting instead.
2. The same as 1. but instead of the full tooling support, apply only the changes from the tooling CL that remove the parts of the existing -shared flag support that can be replaced by external linking. This will ease the burden of maintaining the shared library changes externally, and the removed code is almost guaranteed not to be needed for the eventual shared library code.
3. Some kind of commitment to include shared library support in Go 1.3. With the discussion dead, I would appreciate some kind of indication of how long I have to maintain the CLs unofficially and when to expect progress.
External contributers
I often see people on the mailing lists mention that Go is a true open source project with many external contributors and that it is not simply a Google project with a public source code repository. I also see them encourage external contributors if they have an itch to scratch. However, I can’t help being slightly put off by my experience so far with contributing, mainly because my CLs take a long time to be reviewed. I do see CLs submitted from external contributors that haven’t been part of the project for years, but they’re mostly small or trivial.
I won’t speak for others, but I would appreciate it tremendously if something was done to lower the lag between hg mail and the review. I have no problem waiting days or a few weeks, but I don’t think a month and more for CLs that follow agreed design is reasonable, even for larger changes. Of course, my CL sample size is small, so I apologize if the delays are caused by special circumstances I’m unaware of.
Finally, I’d like to suggest the codereview plugin be expanded to support patch queues or alternatively to work with the hq mq extension. I can see why the core devs don’t need mq because their CLs are applied soon after creation, but for me, hq mq was essential to manage up to 6 levels of separate but dependent patches. Converting and updating mq patches to CLs is an error-prone and time consuming process.
I would file an issue for this, but I noticed that mq was indeed supported at some time but was since removed. So I’m unsure whether patch queue support is acceptable in the codereview plugin at all.