text:
Error on the command scons platform=linux

Tested versions
reproducible in 4.2.stable
System information
Godot v4.2.2.stable - Windows 10.0.19045 - GLES3 (Compatibility) - ANGLE (AMD, AMD Radeon HD 5570 (0x000068D9) Direct3D11 vs_5_0 ps_5_0, D3D11-8.17.10.1433) () - Intel(R) Pentium(R) CPU G2030 @ 3.00GHz (2 Threads)
Issue description
Hi, I'm working on GDExtension, when I run the command scons platform=linux, I get the error.
I don't know if it is a bug or if I miss something.
PS C:\Users\Dexter\Documents\gdextension_cpp_example> scons platform=linux
scons: Reading SConscript files ...
Auto-detected 2 CPU cores available for build parallelism. Using 2 cores by default. You can override it with the -j argument.
Building for architecture x86_64 on platform linux
scons: done reading SConscript files.
scons: Building targets ...
Linking Static Library godot-cpp\bin\libgodot-cpp.linux.template_debug.x86_64.a ...
scons: C:\Windows\System32\cmd.exe: No such file or directory
scons: *** [godot-cpp\bin\libgodot-cpp.linux.template_debug.x86_64.a] Error 127
scons: building terminated because of errors.

Steps to reproduce

create a GDExtension
run the command scons platform=linux

Minimal reproduction project (MRP)
N/A

If this is only reproducible when building godotengine/godot-cpp you should open an issue there instead

If this is only reproducible when building godotengine/godot-cpp you should open an issue there instead

Sorry my bad


text:
Godot freezes when doing basic things such as saving project.

Tested versions
4.2.2
System information
Godot v4.2.2.stable - Windows 10.0.22631 - Vulkan (Mobile) - dedicated Radeon RX 560X Series (Advanced Micro Devices, Inc.; 31.0.21912.14) - AMD Ryzen 5 3550H with Radeon Vega Mobile Gfx (8 Threads)
Issue description
When I save a scene it freezes, causing me to close godot and restart. It has also happened when trying to run the project to test.
Steps to reproduce
not sure how to reproduce other than save, I think this has to do with my system specs/hardware
Minimal reproduction project (MRP)
N/A

Seems to work fine with 4.3, thanks.

source: gharchive/issue

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
TestAudioStreamWAV: use runtime load instead of importer

#93831 introduced a way to load WAVs at runtime. The test has been modified to use that instead of an importer instance.

Thanks!

source: gharchive/pull-request

authors: DeeJayLSP Repiteo


text:
Fix Ctrl+click on @GDScript constants

Fixes #17958
It feels a little sloppy to hardcode the four constants, but I haven't found a more elegant way to get these names yet. In modules/gdscript/gdscript.cpp they are added as globals, but seem to be lumped in with all the other global values. Maybe I can add a list to GDScriptLanguage that keeps track of these constants?

Thanks, hardcoding those is indeed a bit hacky but it's good enough for the time being IMO :)

authors: akien-mga flashyincceo


text:
Restore non-WebAssembly (asm.js) support in HTML5 export

This reverts commit 9107357c8dfda98c5adb33d3c5f4a7be1996fa07, reversing
changes made to 0aa4765904b0aea28ccf485b2428b027a5960df4.
Basically what the title of the commit says, plus modifications to make it work against latest master again.
When building the javascript export template you can say:
wasm=yes for WebAssembly or
wasm=no for asm.js support.
Tested with a 2D Example scene, both variants exported, working in Firefox.
Chromium has some OpenGL related issues though, but those happen in master too and should be unrelated to the exporter itself.
Furthermore, you can export to HTML5 now if you ONLY have specified atleast a debug or a release template (means this is enough already, in case you dont have "regular" templates installed.

Forgot to mention, it will be particularly useful with the upcoming WebGL 1 support, especially for devices which dont support neither WebAssembly nor WebGL 2, like safari on ios.

Yes, would definitely like to see asm.js as autoloaded fallback instead of a separate export option. Wasnt even sure why it was removed at all. Just because WebGL 2 is a requirement in general ? This is about to change with 3.1, re-introducing GLES2 / WebGL 1 support.
Does iOS 11 support WebAssembly then without any issues ? Cant test here because my iOS 10 device (IPhone SE , the one which has the audio jack still) is needed as test device for another iOS project which needs iOS 10 support, so i still hesitate with upgrading. Surely, not the best reason, but would the effort be that high to "just" add a couple of more linker flags for emscripten ? And furthermore, i would like to do a project in godot which is about to run on mobile platforms, but i dont want to mess with the Appstores, especially not with the one from Apple. It has been very nasty to get things published there in my other project, so... bad experience with that. So i was thinking HTML5 based could be a way to do it.
And Startup time with a simple scene is about 10 seconds (2D, a further experiment with the gles2 branch and a simple 3D scene about the same, tested on safari on iOS 10.2.1. Guess it is due to loading a couple of MB of javascript.

Supporting asm.js doesn't mean just adding some lines to the build
scripts, it implies maintaining an additional build target.
Any changes, especially build system changes, have to be tested twice,
with WA and with asm.js.
Additionally, asm.js has exclusive flaws that occasionally require
workarounds: single-precision floating point numbers are not supported,
64-bit integers are slow,...
We're also reintroducing the problem of fixed memory size, a problem we
don't have with WebAssembly for now.
The version of WebKit (i.e. Safari) that comes with iOS 11 does support
WebAssembly, so restoring asm.js support for Apple devices is not worth
the trouble.
On 2018-05-16 2:50 PM, scorpion81 wrote:

Yes, would definitely like to see asm.js as autoloaded fallback instead
of a separate export option. Wasnt even sure why it was removed at all.
Just because WebGL 2 is a requirement in general ? This is about to
change with 3.1, re-introducing GLES2 / WebGL 1 support.
Does iOS 11 support WebAssembly then without any issues ? Cant test here
because my iOS 10 device (IPhone SE , the one which has the audio jack
still) is needed as test device for another iOS project which needs iOS
10 support, so i still hesitate with upgrading. Surely, not the best
reason, but would the effort be that high to "just" add a couple of more
linker flags for emscripten ? And furthermore, i would like to do a
project in godot which is about to run on mobile platforms, but i dont
want to mess with the Appstores, especially not with the one from Apple.
It has been very nasty to get things published there in my other
project, so... bad experience with that. So i was thinking HTML5 based
could be a way to do it.
And Startup time with a simple scene is about 10 seconds (2D, a further
experiment with the gles2 branch and a simple 3D scene about the same,
tested on safari on iOS 10.2.1. Guess it is due to loading a couple of
MB of javascript.

Agree with @eska014, this is too much effort for too little gain, platform support for WebAssembly seems good enough.
We do lose the <= iOS 10 people, which means all devices that can't upgrade (all up to and including iPhone 5). But at this point, I don't think re-adding support is worth it.

We do lose the <= iOS 10 people, which means all devices that can't upgrade (all up to and including iPhone 5). But at this point, I don't think re-adding support is worth it.

It's important to note that we only lose those users for the HTML5 export, which is still not the prime medium to bring games to such platforms. People who want to support those platforms should do native apps, the asm.js export was never stable enough to replace native apps anyway (wasm is much better).
Based on the above, closing this PR as it's more work than gain if we readd it.


text:
Do not use deprecated stuff that only glibc now implements

Since C99, finitef, isnanf and friends have been deprecated, use the standard replacements on linux too.
Fixes #20036

Note: this PR has been proposed upstream, as mentioned in the related issue.
It might be worth waiting for the change to be made upstream.

It was merged upstream, merging here too. Thanks for the great open source work :)

Cherry-picked for 3.0.7.

authors: akien-mga groud ronchaine


text:
Removed unused code

Fixes #15667
From what I can tell, everything the warnings point to is unused code.
The last warning that still shows up is:
[modules/visual_script/visual_script.cpp:1718] -> [modules/visual_script/visual_script.cpp:1716]: (style, inconclusive) Found duplicate branches for 'if' and 'else'.

But can be ignored since VSDEBUG is simply nothing:
https://github.com/godotengine/godot/blob/1093c0ff51b980634dffdd9618eaa53061da6419/modules/visual_script/visual_script.cpp#L1441
Which makes the following if-else an obvious duplicate branch that probably gets optimized out by the compiler anyway:
https://github.com/godotengine/godot/blob/1093c0ff51b980634dffdd9618eaa53061da6419/modules/visual_script/visual_script.cpp#L1716-L1720

PropertyEditor has been removed in #21974 so it can also be left out here. Not sure about the GLES3 method, it's unused but I guess it was written for a reason, CC @reduz.

Pull request updated

Thanks!

authors: Rubonnek akien-mga


text:
Tweak some editor texts' capitalization, use "Go to" instead of "Goto"

This also fixes a typo in the debugger/debug_with_external_editor shortcut. (Customizations of that shortcut will be no longer effective, but it's easy to change it back.)

Thanks!

authors: Calinou akien-mga


text:
[WIP] Fix physics trimesh stability and minor issues

Fixes #21409
Fixes #21341

Closes #22679

Now it's finished can be merged

Thanks!

authors: AndreaCatania akien-mga


text:
Fix "Show in File Manager" option

This pull request fixes "Show in File Manager" option for elements added in the favorites section.
This pull request fixes #27721

Thanks!

Cherry-picked for 3.1.1.

authors: akien-mga danielkulas


text:
Clarify get_node vs get_node_or_null.

Fixes https://github.com/godotengine/godot-docs/issues/2771. It is confusing to say get_node will "raise an error" if
the node does not exist, as this phrase is used elsewhere in the godot
docs (e.g. at several points in the GDScript overview) to indicate
something that will completely stop execution. Saying it logs an error
is more accurate.

Thanks!

authors: akien-mga rcorre


text:
Shortens minimum height of script editor

Changes the minimum height of script list and member list windows from 90 to 60.
Before this PR, the minimum height pushes the bottom panel off screen on a 13-inch monitor when the Audio panel is opened. To reveal the bottom panel toggle buttons, I have to switch to another editor (2D / 3D / AssetLib), and adjust the window height.

The minimum height of the two item lists now looks like this:

Thanks!

Cherry-picked for 3.2.1.

authors: akien-mga timothyqiu


text:
Prevent crash when awaiting in a getter/setter

Fixes #51588
Probably not a proper fix, because getters with await can't be called (still better than editor crash). Setters will work properly.

Thanks!

authors: KoBeWi vnen


text:
Use Enter instead of F2 as an editor renaming shortcut on macOS (3.x)

3.x version of https://github.com/godotengine/godot/pull/54924.
Tested locally by using KEY_ENTER as a shortcut in the Scene tree dock and FileSystem dock, it works as expected. The same shortcut override can't be used in the project manager due to a conflict though.
This closes https://github.com/godotengine/godot/issues/54923.

Thanks!


text:
Allow the drawing and editing of all animation bezier curves

Based on godotengine/godot-proposals#3141.

Moves the bezier edit button from being a property of a track to the animation editor's toolbar
Draws all bezier curves simultaneously and allows them to be selected simultaneously. Only one track remains 'primary selected', which affects adding new keyframes, or prioritizing keyframes when they overlap.
Can click on a curve to select its track
F zooms in to encompass the selected key(s) and the previous + next (based on Maya's graph editor)
Tie the track editor's filter view to bezier editor, hiding all tracks that are not part of that object.
Ctrl+A selects all keys that can be selected (lock/hide/filter to prevent selection), Shift+Ctrl+A deselects all keys.
Tracks can be hidden and/or locked. Adds a solo-button as well to hide all but that particular track.
Fix editor visual bugs. Changes in the way 4.0 drew some stuff, like the introduction of TextLine, wasn't taken into account, so visually it was starting to break in a few places.

https://user-images.githubusercontent.com/2823241/142140926-6ebba3d3-a9a9-49ac-a9e7-66744223f1f4.mp4

Great work so far :slightly_smiling_face:

Ctrl+A selects all keys that can be selected. Lock/hide/filter to prevent selection

Could Ctrl + Shift + A be added to deselect all keys?

Could Ctrl + Shift + A be added to deselect all keys? This would complement Ctrl + A well.

Sure!

Some bugs I found testing. I'm not sure they're all related to this PR specifically.

First there's the issue when the icons overlap the layer names, the text should get clipped somehow.

After creating bezier tracks, say, for position, trying to insert a key from the inspector or the toolbar loc/rot/scale key icon gives the following error:

ERROR: Condition "arr.size() != 6" is true.
   at: track_insert_key (scene/resources/animation.cpp:1628)

Also, it only inserts the key for one of the channels at most. The picture below shows a key inserted only for the position:y component after clicking the key icon next to Transform -> Position in the Inspector.

The X and Y axis colors are inverted in 2D: Keying position:x draws a curve in green and keying position:y draws a curve in red.

First there's the issue when the icons overlap the layer names, the text should get clipped somehow.

Yeah, I wanted to grow the timeline to some minimum size, but I don't have access to the timeline width from the bezier editor. Clipping is likely the better solution in the interrim.

After creating bezier tracks, say, for position, trying to insert a key from the inspector or the toolbar loc/rot/scale key icon gives the following error:

ERROR: Condition "arr.size() != 6" is true.
   at: track_insert_key (scene/resources/animation.cpp:1628)

Weird. I'll look into that. The array size being wrong seems to imply it sent a key without handle_mode being taken into account.

Also, it only inserts the key for one of the channels at most. The picture below shows a key inserted only for the position:y component after clicking the key icon next to Transform -> Position in the Inspector.

Will have to take a look, thanks.

The X and Y axis colors are inverted in 2D: Keying position:x draws a curve in green and keying position:y draws a curve in red.

The colors are calculated from the hash of their property path rather than their axes, so it's always been the case. Would have to somehow detect that it's coming from position/rotation/scale to do axis coloring.

Highlighted bugs and some mentioned in private conversation should be fixed, and I swapped the lock icons around to act like the visibility icon.

Just tested everything again, it's all working well so far.

Note: Currently the Bezier track insertion from the Inspector is broken, but this PR might fix it.

Note: Currently the Bezier track insertion from the Inspector is broken, but this PR might fix it.

I've added keys from the inspector in the process of testing, so it should work.

looks good to me!

Needs a rebase to solve merge conflicts.

@akien-mga Gone ahead and added your suggestions. Thanks :)

Thanks!

Doesn't the "bezier_edit" signal in AnimationTrackEdit is no longer needed?

Doesn't the "bezier_edit" signal in AnimationTrackEdit is no longer needed?

You're right that it is no longer in use. I've gone ahead and removed it in #57920

Due to a change in the _key_selected() argument, key selection in AnimationTrackEditor is broken.

authors: Calinou NathanLovato Razoric480 TokageItLab akien-mga reduz


text:
Use correct #include for time.h on FreeBSD

Use correct #include for time.h on FreeBSD
Module basis_universal uses linux <sys/timex.h> header instead
FreeBSD <sys/time.h> due to missing preprocessor comparison.
Fixes  #55138

Third-party files should not be modified without also including a .patch file. I recommend opening a pull request upstream so that other users of the library can also benefit from the fix :slightly_smiling_face:

Thanks for your effort. I will try to do that way from now on with thirdparty modules issues. Should I close this PR then?

Closing, as https://github.com/BinomialLLC/basis_universal/pull/265 was merged and the version included in Godot was updated: https://github.com/godotengine/godot/tree/master/thirdparty/basis_universal

authors: Calinou robfram


text:
Improve mouse input event class reference

The value of global_position being 0,0 if you're not using Control._gui_input was a bug that was fixed at some point. The value not updating if the mouse isn't over the current control is something I noticed while testing that. Closes https://github.com/godotengine/godot-docs/issues/5464

Fixed.

Thanks!

Cherry-picked for 3.5.

Cherry-picked for 3.4.3.


text:
[3.x] Improve TileMap editor grid

This PR implements 4.0-style TileMap grid.

Grid is now only drawn for certain areas and fades in from cells around.

The used rect of TileMap.
The preview rect of Paint tool (cell / line / rect).

The grid now fades out when the drawn cell size is relatively small. Both for performance and visual pleasing.
Grid for TileMaps with half-offset now has a slightly performance improvement.

A grid line with half-offset is drawn in a single draw_multiline_colors() instead of several draw_line() calls.

Adds editor settings to toggle the grid and to change the grid color.

Also fixed some EDITOR_DEF() that should be EDITOR_GET() in tilemap code.
https://user-images.githubusercontent.com/372476/157790026-b81cf427-38ea-447c-80af-2c598a69ec5d.mp4
p.s. The bright yellow lines in the video are part of the TileMap editor. I turned off "Show Origin" and "Show Viewport" when recording.

Thanks!


text:
[ICU] Add library name suffix for API rename to avoid conflicts (GDExtension build).

Same as #59656, but for TextServer GDExtension build.

Thanks!

authors: akien-mga bruvzg


text:
Correct physics support generation for Capsule2D

Fixes #73869
_SEGMENT_IS_VALID_SUPPORT_THRESHOLD is used to threshold dot values in code but 1.0 - threshold is not a correct inverse. As a result of this, condition was never triggered, even in best case scenario like in linked issue test project. That resulted in Capsule2D generating only one support point . Point that was selected was switching sides each 2 physics iteration, which lead to velocities that were above sleeping threshold.

CC @rburing. See #69657 for the previous change to this code (both 2D and 3D).

Indeed the condition abs(n.y) < 1.0 - threshold is much stronger than abs(n.x) > threshold.
Here is a visualization with threshold = 0.99 and 200 unit vectors:

Old: 2 "horizontal-ish" vectors n with abs(n.y) < 0.01
New: 18 "horizontal-ish" vectors n with abs(n.x) > 0.99

I agree checking for a horizontal vector is better done with the condition abs(n.x) > threshold where threshold is close to 1.0 and less than 1.0. (The alternative would be to keep using abs(n.y) < threshold but increase the threshold.)
So I agree with this PR, but we need to do the same in 3D.
You can do that in this PR (and add me as coauthor) or I can do it in a follow-up PR.

@rburing Updated commit to fix all cases in 2d and 3d shapes

authors: Black-Cat akien-mga rburing


text:
Propagate error correctly when max texture size for lightmaps is too small.

Add error handling for BAKE_ERROR_LIGHTMAP_TOO_SMALL, which was previously ignored. Fixes #81453.

Thanks!

Cherry-picked for 4.1.2.

authors: DarioSamo YuriSizov akien-mga


text:
Fix enum autocompletion for core classes

While writing a gdscript, autocompletion wouldn't suggest Enum names for core classes and also wouldn't suggest Enum Values even after the Enum name had been typed. This has now been fixed.
Now, while finding identifiers in a class, enumeration names are also fetched and enumeration constants are now suggested as well.
Fixes #88858.

CC @HolonProduction

Sure thing! I'll be sure to add the locations and try to do some tests.

@HolonProduction just to be sure I understood the autocompletion test syntax. A very simple test could look like this:
enum_values_autocompletion.gd
extends Control

func _ready():
    AStarGrid2D.Heuristic.➡

enum_values_autocompletion.cfg
[output]
include=[
    {"display": "HEURISTIC_MAX"},
    {"display": "HEURISTIC_OCTILE"},
]

Please feel free to correct me, I'll be sure to fix anything before committing any new changes.

@HolonProduction sorry to bother again, but I'm not sure on how I should be testing the value of location. In my test, I'm autocompleting a node of type BaseButton (which inherits from Control) and my .cfg looks like this:
[output]
include=[
    {"display": "DrawMode",
     "location": },
    {"display": "Anchor",
     "location": },
]

In this case, the location of DrawMode should be LOCATION_OTHER and the location of Achor should be LOCATION_OTHER + 1, but it seems that I can't directly define "location" as LOCATION_OTHER in the .cfg.

In this case, the location of DrawMode should be LOCATION_OTHER and the location of Achor should be LOCATION_OTHER + 1, but it seems that I can't directly define "location" as LOCATION_OTHER in the .cfg.

From the code LOCATION_OTHER is 1 << 10 or 1024.

Thanks!

authors: HolonProduction akien-mga dcaoc03


text:
Improve the script instance binding API

As explained on IRC, there are two things wrong with the current API:

ScriptLanguage::alloc_instance_binding_data has no context about what to allocate. Instead, we should call set_script_instance_binding(int lang_idx, void* data). The caller is in charge of allocating the data.
We need to know if there is data already attached. Either get_script_instance_binding should return null if there is nothing allocated, or there should be a has_script_instance_binding method. I chose the former with this PR.

This is kind of how my code looked with set/get_meta: snippet
This is how it should look with this API: snippet

nope


text:
Removed duplicate metrics and added in the common file

Removed duplicate metrics from the pubsub package and added those in the common file(metrics.go).

Github issue: https://github.com/gofr-dev/gofr/issues/89

@VipulSharma1999 please check the linters, the code quality check is failing

repo: gofr-dev/gofr


text:
Please support common management endpoints in particular /health

1. What version of Go and system type/arch are you using?

go version go1.18.4 darwin/arm64
2. What version of GoFrame are you using?

v2.1.4
3. Can this issue be re-produced with the latest release?
yes
4. What did you do?

searched all the docs.
5. What did you expect to see?
support common management endpoints such as /health /ready or something else. Better to see custome indicator can be registed to these endpoints.
6. What did you see instead?
404 Not Found

It is recommended that you can implement it yourself in routing.
You must know that the standards for health detection are different in different business scenarios.
You can use *ghttp.RouterGroup as route grouping.

If I setup a health check endpoint, how to exclude this endpoint from access logs? @zhonghuaxunGM

@chamhaw
I don't understand why you do this.
The health check endpoint is also a kind of API. If you want record the result of apis which were accessed, why you exclude that ?

If you still has question, plz let  me know.

authors: chamhaw zhonghuaxunGM

repo: gogf/gf


text:
Fix CLI docs formatting/summary

The content in the below shown blocks looks to ignore all formatting of the underlying markdown file and render as plain text
It also includes the word "synopsis" as part of inline content.
This does not seem correct.

The page at https://gohugo.io/commands/ displays a text-only, no linebreaks summary of the richly formatted command pages.
Here's an example command page shown in the docs: https://gohugo.io/commands/hugo/
Here's the source that drives the command page: https://github.com/gohugoio/hugo/blob/master/docs/content/en/commands/hugo.md
Other "index" pages don't seem to have this problem, and also seem to use rich text. For example: https://gohugo.io/templates/

The appears to be because the YAML frontmatter for the templates page contains a description section, for example:
https://raw.githubusercontent.com/gohugoio/hugo/master/docs/content/en/templates/introduction.md
"description: Hugo uses Go's html/template and text/template libraries as the basis for the templating."
It seems like the only thing needed to fix this issue is adding description fields to the commands documents.

The list template fills the boxes using the boxes-section-summaries partial. That partial uses .Param.description (piped to markdownifiy) with a fallback for .Param.summary (no pipe). The page template includes .description at the top, which means that it would be duplicated if that's used.
However, these pages are auto-generated. Last line in the content files:
###### Auto generated by spf13/cobra on 13-Sep-2020
I suspect that the "docshelper" has something to do with that but I haven't figured out how (yet).
My proposed changes would be:

include summary in the front matter. Use either synopsis up to first code section or the intro line ahead of synopsis
replace the text now in summary with shortcode <<{ param "summary" >}} in the content to avoid duplication

Further potential enhancements unrelated to this issue:

add 'toc: true' in the front matter for consistency with the rest of the docs
change capitalization of 'See also' section to match the rest of the docs style.

I recommend that this be closed as a "won't fix".
As a learning experience I tracked down the docs generation into the spf13/cobra package. The GenMarkdownTreeCustom command is what hugo relies on to generate the docs. (cobra code blob). The "prepender", which generates the front matter only has access to the file string, not any of the content. That means there is no easy way of modifying the front matter without overloading this function (I'm not a GO programmer, so not sure if that's possible). Overall, too much effort for what it's worth.
It is possible to "clean" the page by inserting summary: " " into the prepend code which lives in the hugo code itself. I personally don't think that looks any better because now the reader must click to learn anything.

I'll shortly create a pull request that resolves the matter but as far as I could figure out it cannot be done without what I consider a hack. It involves parsing the content with string search to identify the relevant content portion to include in the summary box. It is significantly less elegant than the rest of the documentation templates. As such, I stand by my advice to close the issue (and discard my upcoming pull request).
PS: is the etiquette to update my previous comment or leave these as separate ones?

authors: DanielVF emelens sebastiaan-lampo

repo: gohugoio/hugo


text:
resource/image: fix "always false" condition

Found using gocritic linter.

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

authors: CLAassistant Quasilyte


text:
Weaver Docker Image

Publish Weaver Docker Image to docker hub
https://hub.docker.com/u/gojektech

Weaver docker image is pushed here: https://cloud.docker.com/u/gojektech/repository/docker/gojektech/weaver, Need to setup Ci for the same

authors: gowtham-sai rShetty

repo: gojektech/weaver


text:
go/build: creating vendor/a/b/c implicitly vendors a and a/b too

The AllowVendor logic in (*build.Context).Import checks for a directory called vendor/a/b when importing a package "a/b".  If only package "a/b/c" was vendored, the directory a/b must exist but will be empty, causing Import not to find the real (unvendored) package.
Import should check for non-empty directories.  This logic differs from the usual (non-vendored) case.

I'm going to mark this as Go 1.6, as it's blocking x/tools/cmd/bundle from being able to update the std copy of http2 from x/net.

Hmm. The vendor spec is very clear:

If there is a source directory d/vendor, then, when compiling a source file within the subtree rooted at d, import "p" is interpreted as import "d/vendor/p" if that exists.

But obviously that's not good enough. I'm surprised this is the first we've noticed this. I can't decide if that's good.
The a/b directory in your example is non-empty: it contains c. It's not entirely clear what to do. I guess if there are no .go files at all (not even ones with exclusionary build tags) then we could ignore the directory. I'll try that.

FWIW, this was an effective fix:
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -581,7 +581,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
                                        vendor := ctxt.joinPath(root, sub, "vendor")
                                        if ctxt.isDir(vendor) {
                                                dir := ctxt.joinPath(vendor, path)
-                                               if ctxt.isDir(dir) {
+                                               if entries, _ := ctxt.readDir(dir); len(entries) > 0 {

@alandonovan, how does that work? Doesn't a/b contain a/b/c? Am I misunderstanding what readDir does? It looks like it calls ioutil.ReadDir. Maybe you overrided the default with a broken ctxt.ReadDir function set that doesn't return subdirectories?

See my EDIT: I don't yet know why it seems to work.

I still can't explain it.  Consider my patch retracted, and sorry for the noise.

authors: alandonovan bradfitz rsc

repo: golang/go


text:
Panic issues with go vet ./...

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.7rc5 darwin/amd64

What operating system and processor architecture are you using (go env)?

Operating system: macOS Sierra 10.12 Beta
Processor architecture: amd64
$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/zg/work"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7rc5/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7rc5/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lw/fyqtdrzj047c4rwz6_xz76180000gn/T/go-build086129730=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?
I ran go vet ./... and I got a panic from what appears to be something on the internals of Go.

What did you expect to see?
Issues with my own code.

What did you see instead?

$ go vet ./...
panic: inconsistent import:
        func strings.HasPrefix(s string, prefix string) bool
previously imported as:
        func strings.HasPrefix(s string, prefix string) bool
 [recovered]
        panic: inconsistent import:
        func strings.HasPrefix(s string, prefix string) bool
previously imported as:
        func strings.HasPrefix(s string, prefix string) bool

goroutine 1 [running]:
panic(0x273480, 0xc420159600)
        /usr/local/Cellar/go/1.7rc5/libexec/src/runtime/panic.go:500 +0x1a1
go/types.(*Checker).handleBailout(0xc4200de1c0, 0xc4207ab8d8)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/types/check.go:213 +0xae
panic(0x273480, 0xc420159600)
        /usr/local/Cellar/go/1.7rc5/libexec/src/runtime/panic.go:458 +0x243
go/internal/gcimporter.(*importer).declare(0xc4202a0750, 0x3f5180, 0xc4202b3cc0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/internal/gcimporter/bimport.go:175 +0x17c
go/internal/gcimporter.(*importer).obj(0xc4202a0750, 0xfffffffffffffffb)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/internal/gcimporter/bimport.go:203 +0x4a2
go/internal/gcimporter.BImportData(0xc420088cf0, 0xc420390000, 0x1938c, 0x1fe00, 0xc4201aa0e1, 0x7, 0x0, 0x7, 0x0, 0x0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/internal/gcimporter/bimport.go:88 +0x390
go/internal/gcimporter.Import(0xc420088cf0, 0xc4201aa0e1, 0x7, 0x7fff5fbff97e, 0x6, 0xc420152730, 0x0, 0x0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/internal/gcimporter/gcimporter.go:166 +0x551
go/importer.gcimports.ImportFrom(0xc420088cf0, 0xc4201aa0e1, 0x7, 0x7fff5fbff97e, 0x6, 0x0, 0xc420347cc0, 0x0, 0x0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/importer/importer.go:70 +0x67
go/types.(*Checker).collectObjects(0xc4200de1c0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/types/resolver.go:191 +0x826
go/types.(*Checker).checkFiles(0xc4200de1c0, 0xc4201bd1c0, 0x3, 0x4, 0x0, 0x0)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/types/check.go:225 +0xaa
go/types.(*Checker).Files(0xc4200de1c0, 0xc4201bd1c0, 0x3, 0x4, 0xc4201b7470, 0xc4200ef960)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/types/check.go:218 +0x49
go/types.(*Config).Check(0xc4201bb780, 0xc420087518, 0x6, 0xc4200a73c0, 0xc4201bd1c0, 0x3, 0x4, 0xc4201be050, 0x0, 0xc4200001a0, ...)
        /usr/local/Cellar/go/1.7rc5/libexec/src/go/types/api.go:344 +0x1a3
main.(*Package).check(0xc4200b2840, 0xc4200a73c0, 0xc4201bd1c0, 0x3, 0x4, 0xc4201bd1e0, 0x2)
        /usr/local/Cellar/go/1.7rc5/libexec/src/cmd/vet/types.go:76 +0x370
main.doPackage(0x2cd6a4, 0x1, 0xc42000a250, 0x3, 0x3, 0x0, 0x1)
        /usr/local/Cellar/go/1.7rc5/libexec/src/cmd/vet/main.go:336 +0x999
main.main()
        /usr/local/Cellar/go/1.7rc5/libexec/src/cmd/vet/main.go:244 +0x345
exit status 2

I wasn't able to reproduce this issue...

If it went away after a go install, this is the same as #16112 and it's not a bug, just a caused by out-of-date .o files and the fact that there's a new export format.

Sounds like a dupe of #16112. Interesting that here we have the exact same pretty-print of the duplicate symbol, though. Please try starting from a fresh GOPATH, and comment here if you're able to reproduce.

authors: ALTree quentinmit zg


text:
Proposal: New build tag for illumos

Disambiguating solaris vs illumos
SunOS/Solaris has a storied and complicated history.  In [2010] Illumos forked from OpenSolaris and has continued its life in the open (, and now spent the majority of its life, as Illumos (not Solaris).
The solaris build tag is "mostly" compatible with Illumos and Illumos-based distributions (e.g. SmartOS, Nexenta, Open Indiana, Delphix, etc), however Illumos has diverged significantly from Solaris.  In order to detect and support Illumos-native functionality, I propose:

illumos be added as a new GOOS build tag
The illumos build tag be distinct from solaris.

We considered extending the life of the solaris build target to include the illumos target for the period of one release but decided against this because it would taint community code with:
// +build !solaris
// +build illumos

that would need to be cleaned up at the end of the transition period.  Backwards compatibility for the sake of backwards compatibility isn't something we're interested in maintaining.

Semi-related: it would be nice if there was a way of specifying and targeting distributions at build time.  cgo on Linux and alpine vs glibc comes to mind as another area that would benefit from a distribution-specific build target.

/cc @4ad @binarycrusader @jtsylve for opinions too.

Can you outline the differences relevant to the Go standard library between solaris and illumos?

Illumos didn't fork in 2007, which would be especially hard since OpenSolaris didn't have it's first release until 2008.   I suspect the submitter meant 2010.
However, many of the other points made are true -- for reasons that are beyond the control of engineering, the OpenSolaris program was discontinued in 2010.  There are roughly seven years of divergence at this point.  Solaris has changed significantly in that time for any new interfaces that were added or updated after the initial fork, etc.
Both Solaris and OpenSolaris-based derivatives (such as Illumos) added or updated existing interfaces after the fork; these interfaces, even though they have the same names, may not be compatible as they sometimes use different constant values in system headers, accept a different number of parameters, or one version accepts values the others does not.
With that said, any interface that existed before the fork will generally be compatible because Solaris has strong guarantees for backwards compatibility at the binary level (although this is not strictly guaranteed at the source level).  The only catch would be new parameter values that functions may not have supported in an older release.
For the vast majority of the Go standard library, there are no appreciable differences -- they can continue to be treated as equivalent platforms.  This is part of why I have not yet approached the Go development team about trying to resolve this.  However, there are some important differences, most of which affect packages such as golang.org/x/sys/unix or anything that uses native system interfaces.
I've mainly been waiting for the right time to approach the golang-dev mailing list about how we should account for the differences since they are starting to matter.  For example, if you run the mkall.sh script in syscall, you'll find a number of differences:  https://gist.github.com/binarycrusader/1c57088d65c8f4071853af5efa37271e
Looking over those differences, you'll see what I said earlier matches up -- some of the error constants are different, Solaris has many that Illumos does not, and Illumos has a few options Solaris intentionally doesn't support (such as MAP_32BIT for mmap()), etc.  The next release of Solaris also supports the xpg7 standard, which Illumos doesn't support yet as far as I know, so there are some new interfaces there as well.
More important differences in constants are things such as TCP_KEEPCNT, etc. that both Illumos and Solaris support but have different values for.
The small remaining differences are things such as different text for particular errnos, or the Bits in FdSet being uint64 instead of int64 on Solaris (bug fix) and don't typically matter in practice.
Now as for the pull request that was referenced, yes, there are big differences there.  Solaris does actually have all of those statistics available, but using a completely different set of interfaces.  Although both Solaris and OpenSolaris-based derivatives have the kstat general interface, the set of stats available is different.  Additionally, the next release of Solaris has a completely new statistics subsystem, so being able to differentiate between Solaris and OpenSolaris-based derivatives is likely important.
There are other considerations as well when adding new interfaces to packages such as golang.org/x/sys/unix; by a quick and hacky estimation, Solaris libc has easily hundreds of additional private/public interfaces not present in OpenSolaris-based derivatives and OpenSolaris-based derivatives have a few that Solaris does not as well.  Generally speaking, those differences won't affect the Go standard library, the primary points of contention are around networking-related interfaces, and some memory-related interfaces (such as mmap).  There are other differences as well when it comes to the system linker (/usr/bin/ld) and what it supports, etc. but I don't think those are particularly relevant (yet).
In short, I've only recently become concerned that this is a problem with solving somehow, and the amount of divergence that affects Go itself is fairly small.
For the record, Go is important to Solaris, and we've tried hard to remain compatible with OpenSolaris-based derivatives.  I've spent roughly two years now working on porting Go to Solaris and on a port of Go to sparcv9 with Aram which should be ready to integrate in the Go 1.10 timeframe.  As such, we're very interested to workout an acceptable solution with all parties involved.

@ianlancetaylor There are a number of new syscalls that only exist in Illumos.  A quick of changes includes:

brand(2)
getrandom(2)
epoll(2)
preadv(2) and pwritev(2)

[/me deletes most of his reply, thank you @binarycrusader for that excellent reply - my history with Go on Illumos is only a handful of months.  I used 2007 as the date based on the head of a series of reflog entries and my memory of the acquisition, but 2010 is the correct date. ]
Internally we were concerned about maintaining portability but know these are going to continue to diverge, but given our use of Go, it's clear that this will be problematic going forward as additional syscalls show up in x/sys/unix.  I'm going to hand-wave past any already present divergence that I suspect is being carpeted over by the fact that we rebuild Go internally and there isn't much crossover between binaries created on Illumos and run on Solaris (though I have heard several anecdotal reports of some binaries not working but not verified this myself).  I'm sure some of the kstats referenced work, though others don't (yet?/ever?).
I think there is a lot of history and shared work that could be leveraged going forward, too, but fundamentally the two OSes have diverged to the point that there is a need for a discrete build tag in order to make appropriate decisions at compile time.

getrandom(2)

Solaris 11.3 added this:
https://docs.oracle.com/cd/E86824_01/html/E54765/getrandom-2.html
As for the other functions, they are not yet available in Solaris (intentionally or because they haven't been implemented yet).  But yes, those are good examples.

Having a way to distinguish illumos and Oracle Solaris targets based on build tags seems fine. I don't think that having incompatible solaris and illumos build tags is a good idea. It breaks a lot of existing code for no benefit.
Let's add two new build tags, solaris11x and illumos, and keep the solaris build tag around. By default, the solaris build tag will match both illumos and Oracle Solaris targets, which is what you want in 99% of cases anyway, and it doesn't break any existing code.
When one wants specific support for a particular Solaris variant, either one of solaris11x or illumos build tags is to be used.

At the end of the day something needs to change as it is disingenuousness to users if we keep with the status quo and expect a GOOS="solaris" binary to run on either Oracle Solaris or Illumos.
If Oracle Solaris is moving to its own build tag (i.e. oraclesolaris), then sharing the solaris build tag with illumos seems fine.  I think this is fine, but don'g want to obligate the Oracle community to this.
If Oracle Solaris is not going to use a new build tag, then I would object to having the solaris build tag be inclusive of illumos.
That said, we're decidedly :+1: to having Oracle Solaris get its own, new, and dedicated build tag to resolve any ambiguity going forward.

I dislike bike-shedding, but for the record, I would discourage any build tag containing something that implies a "version" of some sort such as "11x", etc.  While that may seem tempting, the public version is used for marketing purposes and isn't reliable as a way to discern differences in technical interfaces.
In particular, Solaris often "backports" functionality to the previous release while the next release is in development.  As an example, Solaris 11.3 sometimes receives new functionality from what Solaris calls "SRUs" (the monthly updates that contain security fixes and other improvements) that are from the current in-development release.
As such, on Solaris, feature-test based builds are ideal.  Unfortunately, Go's architecture (as far as I can tell) assumes runtime-based feature tests instead of build-time feature tests (unlike rust's build.rs) so that makes this difficult.
In an ideal world, I feel like the original build tag for solaris probably should have been "sunos", with the Sun/Oracle specific variant being "solaris" and "opensolaris" for the community derivatives.  This makes sense too because "Solaris 11.3" is technically the marketing name and marketing version for the current release of SunOS 5.11.
However, since we don't live in an ideal world, I'd suggest "sunos" and "illumos" as the two new tags going forward; they're version-agnostic and reasonably accurate.

I'd suggest "sunos" and "illumos" as the two new tags going forward; they're version-agnostic and reasonably accurate.

This seems like a reasonable suggestion to me for all of the reason that @binarycrusader mentioned in his last comment.

I am very much opposed to a system which tries to maintain compatibility between Oracle Solaris and Illumos distributions in a manner which would differ from every other GOOS. In my opinion we should keep the existing solaris build tag as referring to Oracle Solaris, and introduce illumos.
This reflects the way that the various BSDs operate, and therefore imposes no additional cognitive overhead on people who don't care about operating systems other than Linux and Darwin (which is, let's face it, most people!)
The short-term pain which will be experienced by the Illumos community to go through and add build tags to things outside the standard library is not to be minimised - but on the other hand nor is the cognitive overhead of a three-tier GOOS system on the rest of the world. I think we in the Illumos community should just deal with this short-term problem.

Given there appears to be no debate surrounding the illumos build tag, can we move forward with that?
@binarycrusader , if there is a desire to have Oracle Solaris be tagged with its own build tag, can that be submitted and addressed independently?
@bradfitz , short of a PR, what would you like the next steps to be?

@sean- I'm not requesting a separate build tag for Solaris, I had only suggested it as a way to ease the transition for OpenSolaris-based derivatives.  My employer has sponsored much of the work done on Solaris at this point for Go, and I've pushed hard to maintain compatibility with OpenSolaris-based derivatives.
As such, I leave the decision on build tags up to the Go maintainers.

As such, I leave the decision on build tags up to the Go maintainers.

We (the Go maintainers) are not active Solaris or Illumos users. Ideally we'd prefer if the Solaris & Illumos would agree on a solution that solves the problems at hand. Maybe that's a new GOOS value. (A full fork, e.g. GOOS=freebsd vs GOOS=dragonfly) Maybe that's a build tag only for now.
Is it a goal (or non-goal) for binaries built for Solaris to run on Illumos, or vice versa?

Solaris only guarantees binary compatibility for binaries built on an older version of the operating system so that it can run on a newer version.  Because the standard libraries (libc) and the linkers are now significantly different, it's highly unlikely to work.  With that in mind, I would assert that it is not a goal for binaries built for Solaris to run on Illumos or vice/versa.
Both share a common set of system interfaces (many decades worth), but we're also nearing one decade of divergence.  It's unclear to me what the intent is with Go's build tags vs. GOOS, so I can't say which should be the answer.

@bradfitz I would treat it as a non goal for binaries built with (say) GOOS=solaris to work on Illumos and vice versa, though if they do it would be a happy accident.
IMO things would be best served moving forwards by treating Solaris and Illumos in the same manner as FreeBSD vs DragonflyBSD. Assuming @binarycrusader et al have no problem with that, it seems to be the path of least confusion, though admittedly with some short term pain.

@bradfitz Are you good with that agreement?  It sounds like we have a consensus that everyone is happy with.

We'd like to do Go 1.9beta1 next week, and I don't think we can really do this before Go 1.10.
If we do, GOOS=illumos, we'd need a GOOS=solaris builder first (#15072, which has been open for some time).

@bradfitz I'm not sure on how a Solaris builder can be supplied beyond VirtualBox (perhaps @binarycrusader can help there), but I'm sure we at Joyent can run any necessary Illumos builders without issue. (cc @bcantrill).

We currently run Illumos builders on Joyent already. (but would love help improving our setup, if somebody has some time) It's the Solaris ones where we lack coverage.

Solaris amd64 currently only supports Xen-based virtualization, Solaris-based virtualization (kernel zones), full virtualization (VirtualBox/VMWare), or bare metal provisioning.  It does not have the virtio/virtnet drivers required by GCE (?).

We have 10 VMWare nodes. But I'd rather discuss on #15072.

Based on discussion above, switching to GOOS=illumos for Go 1.10 is fine. There's no need to have Solaris builders before then, although of course if we get further into the cycle with no Solaris builders we might reconsider GOOS=solaris entirely (golang.org/wiki/PortingPolicy). But that's just exposing a current problem (no Solaris builders, only Illumos ones), not introducing a new problem.
-rsc for @golang/proposal-review

There's an Oracle Solaris builder in place now and there will be more soon, so this change can be made when appropriate.

how you want specify how build targets 'solaris' and 'illumos' should be different?
i do DilOS (based on illumos) , and i'm interested in details. i try move to use Debian more Debian userland, but it is not clean to me how you want to split 'solaris' and 'illumos' targets? who will be responsible for feature requests? and we have golang-1.8.x and i'm interested in next updates.

Moving to Go 1.11, as this apparently didn't happen while I was away on leave.

I don't see this mentioned and I'm generally hesitant to chime in on this sort of thing for fear of bikeshedding, but I didn't see this mentioned here.
I think it would be reasonable to target the Solaris 10 brand. The syscall ABI isn't guaranteed to be backwards compatible in the master branch of Illumos (or Solaris, to my knowledge). The libc is generally considered the compatibility boundary.
https://github.com/joyent/illumos-joyent/blob/master/usr/src/uts/common/brand/solaris10/s10_brand.c
The Solaris 10 brand, however, does have some implicit guarantees about the kernel ABI. These may be spelled out explicitly somewhere, I'm not sure.
From the top of my head, I think this approach would allow binaries to run on both Solaris and Illumos without any loss of functionality.
(As a parenthetical footnote, I think it'd be nice if Illumos had a distinct OSABI and attendant host triples etc in compiler toolchains along with a stable kernel ABI, but the general consensus is that the whole issue is a spectacular troll.)

We're not doing Solaris 10, it lacks APIs we use. Even if we were doing Solaris 10, we'd want to use different APIs on Solaris 11, so Solaris 10 can never be a base for everything.

As for the syscall ABI compatibility, that is a non-concern since Go only uses libc on Solaris variants.

What has to be done still to add this?

@Smithx10, it needs ownership. So I don't think this is going to happen anymore. Most of the people who were once behind this have dropped their support.
There's not enough ownership for one Solaris port at the moment, much less two.

@bradfitz   What is required by someone in the community to take ownership of the Illumos tag?  I don't really have a social life and might be able to take on a hobby.

To @Smithx10's point, I think more generally we can find folks in the community to take ownership of this. How do people sign up to help?

@bcantrill @Smithx10 See https://golang.org/wiki/PortingPolicy.

@bcmills   Thank You!   I think ATM I'll take @jclulow 's advice and clone the go source, look at all the places that the "solaris" string comes up and add "illumos" versions of that, and get the tests to pass at least as well as they do now.  Once we get that I can work on the builder.    @bradfitz  If the first iteration doesn't have any changes is that ok?  I think at this time just having the ground work in place would be a big win.  After that, we can start addressing differences,  does that make sense?

@Smithx10 @bcantrill It was tough enough to get the 3rd Parties to support building on solaris by including the appropriate versions of upstream Libraries that had appropriate cross-platform versions of Flock and MMAP that work under illumos. If we switch everything from "solaris" to "illumos" would that mean that Upstreaming work of the last 3 months becomes useless?

@Toasterson I don't think you have to switch anything. Does adding another tag for illumos force anyone to do anything?  Am I missing something?  Sorry,  I'm new to this particular tag issue and just want to maintain the Nomad agent on Illumos and SmartOS.   Please feel free to educate me.

@Smithx10 The Main problem will be vendoring. gitea and the mmap library which is used there come to mind. But we Also have termios to redo the tags and see that all vendored copies are updated. Vendoring has become such a often used tool that a change such as this will take a huge amount of time and effort to push through. Even more when people who develop the app mainly only care for Linux.
I just did that whole work for the last 3 months (not fulltime) and I am asking if I have to do the whole thing again.
What is the Problem with the Nomad Agent If I may ask?

@Toasterson Can you point me at the flock()/mmap() work you're referring to so that I can take a closer look?
I did an illumos port of wireguard-go recently.  I was unable to find a nice way to make calls to ioctl(), getmsg(), or putmsg() -- but I was able to create a private set of wrappers using the same pattern used to wrap libc functions in the x/sys/unix package and it seems to work well so far.  Perhaps notably, it did not require any use of Cgo.

@Toasterson I just ran into trying to get this merged and realized it's blocked on this issue.

We could make GOOS=illumos imply the solaris build tag like we did with GOOS=android implying linux. That would mean most +build solaris-constrained code would still work and people wanting to differentiate could use +build illumos or +build solaris,!illumos.
But we still need people owning the builders for both Solaris & illumos. The Solaris builder owner left the company and it's currently just still ~working by accident, but it's running an old version of the build system. And the one that I kinda run is ill-maintained on Joyent (SmartOS) and if that started being the illumos builder then we'd no longer have a maintained solaris builder.
I was able to run OmniOS CE on qemu-kvm on GCE (in https://github.com/golang/go/issues/15581#issuecomment-435431402) and that's the path (or similar) I'd like to take going forward to run the Solaris (or rather, illumos) builders on GCE where we have ~infinite free quota and fast networking with the rest of our build system.
But that Docker image was so big that I got distracted and started writing https://github.com/google/crfs for better start-up times, which is in progress. Once that's done I'd still need help preparing a script to automate the creation of OmniOS or SmartOS qemu images that run our x/build/cmd/buildlet/stage0 on boot.

@bradfitz For what it's worth, I had a shot at booting SmartOS under GCE the other day.  There were some (uninteresting) panics that I need to work out, but I'm able to drive our kernel debugger (kmdb) through the serial port redirection so I imagine it shouldn't take much to get that sorted out.  I embarked on the exercise so that I could then work on an in-OS virtio-scsi driver so that we'd work well under GCE.
If I'm able to get OpenIndiana/OmniOS/SmartOS images to the point where they work directly under the GCE hypervisor, is that our best path forward with respect to builders for the illumos tag?
I'm not really sure what to do about the solaris tag, to be honest; Solaris and illumos have been diverging for many years now, and any resemblance at this point is increasingly coincidental.

@jclulow certainly
mmap was edsrzf/mmap-go#15 Where I had to push the library to use x/sys/unix. gitea will also still need a push to update as  go-gitea/gitea#4799 is still open and that is the main issue vendoring.
flock was at sevlyar/go-daemon#47
These two plus dependents I know will break if we switch to illumos as tag instead of solaris. Another like src-d/go-billy#65 will still work as the tag is posix and not solaris directly.
github.com/pkg/term/termios will also break
We have been using the solaris tag for us for a while now. Out in the open. We will break stuff for us if we switch.
As for ioctl you mean something like https://github.com/pkg/term/blob/aa71e9d9e942418fbb97d80895dcea70efed297c/termios/ioctl_solaris.go#L6 ? That does require cgo but it is cross-compilaeable because libc can be linked somehow. There is some magic here :)

@Smithx10 That PR looks odd If you ask me. I don't see why the kstat changes needed to be in files named _illumos. Solaris also has kstat. Especially as it is calling the kstat binary and not using any syscall interface.....

@bradfitz Yes that way of tagging would be the way forward.
As for GCE. I have recently begun researching what it would take to build an image analogous to the Official FreeBSD Image on the Marketplace, for circleci. Would that be what you are looking for? Also in OpenIndiana we have a Vagrant Box I don't know If a good image conversion tool exists but we use packer to build that box. So there is automation available.

@Toasterson   Good idea.  I'll take a look and see if I can refactor that to just work under the Solaris tag for now.

I don't have access to Oracle Solaris anymore, but rumors that I don't (or don't want) to maintain the illumos port are greatly exaggerated.
The problem is that running a builder used to be trivial, but now it requires this complex machinery in the cloud. Frankly, the act of running a builder vastly dwarfs in effort and complexity what is required for actually maintaining the port. That's not a great place to be in, IMO.
Doing this change is trivial, but I don't want to do it just yet because I don't want to kill Oracle Solaris support just yet.
I'll be contacting some people to determine the best way to move forward.

The problem is that running a builder used to be trivial, but now it requires this complex machinery in the cloud. Frankly, the act of running a builder vastly dwarfs in effort and complexity what is required for actually maintaining the port. That's not a great place to be in, IMO.

That's a little exaggerated. You can still run a builder on a single physical or virtual machine if you want, almost the same as before We prefer to cloud-ify them so we have elastic capacity, but that's not a requirement. Our Mac builders aren't cloud-ified, for instance.

@jclulow, running directly on GCE isn't a requirement. I'm not asking you or anybody to write a virtio-scsi driver. If you can run on QEMU (which is happy to do virtio-block unlike GCE or IDE/SCSI/etc) and can prepare a VM image for us, we can supply lots of CPU quota. Otherwise you can run it on your own hardware elsewhere.

@bradfitz Would a Vagrant image for Quemu/KVM work or what image would you need and how would one prepare that? We have many different images for VM's etc in different formats already.

@Toasterson, I don't care which specific tools are used. The only requirement is that it's some automation that runs on Linux that outputs a VM image. We've used bash, Go, expect, Powershell (run remotely on Windows VM), Python (Anita), etc.
But again, the bigger concern here is that if we do this split, then who runs the Oracle Solaris builders?

@bradfitz Does illumos require someone to maintain the Oracle Solaris port? When @jclulow has the SmartOS builder running ( https://twitter.com/jmclulow/status/1117305055755919360 ) and a community member takes the roll of maintainer, I'd imagine it would be desirable for the tag to be completely independent from the Oracle Solaris lifecycle.   What do you think?

I'm not going to ask that an illumos contributor maintains the Oracle Solaris port (and its builder).
But it'd be a sad situation if we split the current GOOS in two and one of the halves dies due to neglect.
Does anybody in this bug care about the GOOS=solaris port existing if GOOS=illumos exists? I'm not sure where to find a GOOS=solaris maintainer. My contacts at Oracle dried up and nobody replied to my golang-dev post.

@bradfitz Speaking as an illumos community member, I'm chiefly interested in a GOOS=illumos that allows us to provide support for things like flock() that we've perhaps done differently from Solaris.  The possibility of having GOOS=illumos imply GOOS=solaris seems very attractive, as it would presumably allow us to make immediate forward progress on the differences without throwing out all the software currently working by using the solaris tag today.
I agree that it's something of a conundrum.  I don't think you can even get Solaris 11 for free, at least not with security updates and bug fixes beyond what is in the major release ISO.  I know a few people that work at Oracle and I will ask if they know of anybody inside who might be interested, but I'm not sure there will be anyone.
If it is eventually decided that the GOOS=solaris tag isn't viable anymore due to lack of maintainers, would we still be able to have GOOS=illumos imply it in the future?  I'm not yet deeply familiar with how the overlapping/implying tag stuff works.

If it is eventually decided that the GOOS=solaris tag isn't viable anymore due to lack of maintainers, would we still be able to have GOOS=illumos imply it in the future? I'm not yet deeply familiar with how the overlapping/implying tag stuff works.

Yeah, if GOOS=solaris does get removed, we could keep "solaris" as an additional implicit build tag when GOOS=illumos.

I sent https://go-review.googlesource.com/c/go/+/174083/ as an example CL of what would be required to do this. Anybody want to pick it up, test it, and finish it?
You have a few days before Go 1.13 goes into freeze.

@bradfitz Hi Brad!  I can take a look at it, though that timeline is obviously a little tight as I don't have a strong background on all of the infrastructure here.  When is the exact date of the freeze, and if we miss this round does that mean we're effectively waiting for Go 1.14?  (When is that?)
As an aside, I have a rough prototype of a Virtio SCSI driver and some other changes required to get illumos running under GCE directly.  I'm learning a bit more about how to parcel up and share a useful GCE image that others can consume.

Freezing May 1st. Tree opens again 3 months after that. https://golang.org/wiki/Go-Release-Cycle

Thanks!

@bradfitz I spent some time on this yesterday.  I took your initial CL (thanks for the push in the right direction!) and have added in a bunch of extra changes, in parts using the android/linux split as a guide.
I have been able to cross build an illumos/amd64 toolchain using a stock solaris/amd64 1.12.1 toolchain I had available already.  Using this cross built toolchain, I was then able to bootstrap a native build of the illumos/amd64 toolchain.  All of the tests appear to pass on the final toolchain in this sequence.
I've enrolled my work gsuite account in the Gerrit instance, and I've kicked off the CLA signing process.  My boss has signed it this morning, but I'm not sure how long it will take to show up in the system -- I can't see it yet.
Will I be able to adopt your existing CL and push an updated change to that, or should I start a new CL with the git codereview tool?

@bradfitz I'd really like to get this in before the gate closes on May 1st.  I've pushed the change that I have so far (which I feel is ready to go) to a branch on Github: https://github.com/joyent/go/commit/9ee73d27649568275117bc48764a4679bbf7fbf2
I believe I've done all the right things with the HTTP cookies file and Gerrit, but I still can't appear to submit a CL:
$ git codereview pending -c
illumos 2b8cbc3..9ee73d2764 (current branch, 1 behind)
+ 9ee73d2764
	all: add new GOOS=illumos, split out of GOOS=solaris

	Like GOOS=android which implies the "linux" build tag, GOOS=illumos
	implies the "solaris" build tag. This lets the existing ecosystem of
	packages still work on illumos, but still permits packages to start
	differentiating between solaris and illumos.

	Fixes #20603

	Change-Id: I8f4eabf1a66060538dca15d7658c1fbc6c826622

	Files in this change:
		misc/cgo/test/sigaltstack.go
		misc/cgo/testcarchive/carchive_test.go
		misc/cgo/testcshared/cshared_test.go
		src/cmd/dist/build.go
		src/cmd/dist/main.go
		src/cmd/go/go_test.go
		src/cmd/go/internal/bug/bug.go
		src/cmd/go/internal/imports/build.go
		src/cmd/go/internal/lockedfile/internal/filelock/filelock_test.go
		src/cmd/go/internal/work/buildid.go
		src/cmd/go/internal/work/exec.go
		src/cmd/go/internal/work/init.go
		src/cmd/internal/objabi/head.go
		src/cmd/link/internal/ld/dwarf_test.go
		src/cmd/vendor/golang.org/x/sys/unix/sockcmsg_unix.go
		src/debug/gosym/pclntab_test.go
		src/go/build/build.go
		src/go/build/doc.go
		src/go/build/syslist.go
		src/net/error_test.go
		src/net/fd_unix.go
		src/net/interface_test.go
		src/net/listen_test.go
		src/net/tcpsock_test.go
		src/os/exec/exec_test.go
		src/os/os_test.go
		src/os/removeall_test.go
		src/os/user/user_test.go
		src/runtime/asm_amd64.s
		src/runtime/cgocall.go
		src/runtime/crash_unix_test.go
		src/runtime/internal/sys/gengoos.go
		src/runtime/internal/sys/zgoos_aix.go
		src/runtime/internal/sys/zgoos_android.go
		src/runtime/internal/sys/zgoos_darwin.go
		src/runtime/internal/sys/zgoos_dragonfly.go
		src/runtime/internal/sys/zgoos_freebsd.go
		src/runtime/internal/sys/zgoos_hurd.go
		src/runtime/internal/sys/zgoos_illumos.go
		src/runtime/internal/sys/zgoos_js.go
		src/runtime/internal/sys/zgoos_linux.go
		src/runtime/internal/sys/zgoos_nacl.go
		src/runtime/internal/sys/zgoos_netbsd.go
		src/runtime/internal/sys/zgoos_openbsd.go
		src/runtime/internal/sys/zgoos_plan9.go
		src/runtime/internal/sys/zgoos_solaris.go
		src/runtime/internal/sys/zgoos_windows.go
		src/runtime/internal/sys/zgoos_zos.go
		src/runtime/mem_bsd.go
		src/runtime/netpoll.go
		src/runtime/pprof/pprof_test.go
		src/runtime/proc.go
		src/runtime/rt0_illumos_amd64.s
		src/syscall/exec_unix.go
		src/syscall/sockcmsg_unix.go
		src/syscall/syscall_unix.go

$ git codereview mail
fatal: remote error: PERMISSION_DENIED The caller does not have permission
(running: git push -q origin HEAD:refs/for/master)
/home/jclulow/bin/git-codereview: exit status 128

We're having a bit of trouble with the CLA process.  It seems a departed Joyent employee might have signed a CLA in the past, and we're struggling through the process of adopting/reactivating it at the moment.  Would that prevent my being able to submit to Gerrit?

@jclulow  Whoa!  This is great news! Hopefully @bradfitz can help clear up the CLA before the deadline and Illumos may finally have it's own tag.  Definitely a win for both communities.   Can't wait!

We're having a bit of trouble with the CLA process. It seems a departed Joyent employee might have signed a CLA in the past, and we're struggling through the process of adopting/reactivating it at the moment. Would that prevent my being able to submit to Gerrit?

There are two ways to submit code to Go: using Gerrit, or using GitHub PRs. Both require the same CLA process. We can't accept code without a CLA.
I think you need to join the go-contributors@$YOUR_COMPANY_DOMAIN mailing list.

Thanks, Brad.  I'll follow up with our internal IT folks tomorrow to see if we have one of those.

My guess is that it's a Google Group, so the URL would be:
https://groups.google.com/a/joyent.com/d/forum/go-contributors
or
https://groups.google.com/a/joyent.com/forum/#!forum/go-contributors
... which I think are identical.

@bradfitz, you're a steely-eyed missile man!
I've uploaded a CL: https://go-review.googlesource.com/c/go/+/174457

The Illumos builder was removed due to the upcoming Joyent Public Cloud turndown.
If folks want to maintain support for the illumos port, it needs a builder. See #15581.

I'll take a look!

@jclulow, does it run on GCE yet? (virtio-scsi was WIP last I heard?)

authors: 4ad Smithx10 Toasterson bcantrill bcmills binarycrusader bradfitz hypoalex ianlancetaylor ikozhukhov jclulow jen20 jtsylve rsc sean-


text:
syscall: fs_nacl.go: Link and Rename missing fs mutex lock/unlock

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.9.2 darwin/amd64
Does this issue reproduce with the latest release?
Issue uncovered by inspection.
What operating system and processor architecture are you using (go env)?
Go playground using GOOS=nacl
What did you do?
Per comments at the head of fs_nacl.go:
// Since there is no I/O (everything is in memory),
// the global lock mu protects the whole file system state,
// and that's okay.

and
// Except where indicated otherwise, unexported methods on fsys
// expect fs.mu to have been locked by the caller.

but Link and Rename call these methods without locking fs.
What did you expect to see?
fs calls should be serialized
What did you see instead?
These two are not.
Fix is trivial. I'm happy the needed changes

Fine for Go 1.10, or whenever. Thanks.

authors: bradfitz tenortim


text:
cmd/go: add more options to security whitelist

What version of Go are you using (go version)?
Go 1.10
What operating system and processor architecture are you using (go env)?
MacOS 10.13.2
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/olt/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/olt/dev/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/np/2fhk15fn5qq6fm4j9s3vt_6r0000gn/T/go-build391399925=/tmp/go-build -gno-record-gcc-switches -fno-common"

Debian Jessie 3.16.0-4-amd64
GOARCH="amd64"
GOBIN=""
GOCHAR="6"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/olt/go"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?
Build code cgo code which includes the evaluated output of
// #cgo CXXFLAGS: $(mapnik-config --cxxflags --includes --dep-includes | tr '\n' ' ')

The output of mapnik-config --cxxflags is as follows (for two different versions):
-g -O2 -fstack-protector-strong -Wformat -Werror=format-security -g0 -ansi -Wall -pthread -O2 -fno-strict-aliasing -finline-functions -Wno-inline -Wno-parentheses -Wno-char-subscripts

and
-std=c++11 -stdlib=libc++ -DBOOST_EXCEPTION_DISABLE -fvisibility=hidden -fvisibility-inlines-hidden -Wall -ftemplate-depth-300 -Wsign-compare -Wshadow -O3

What did you see instead?
For the first version:
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -ansi
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -finline-functions

It works with the following env: CGO_CXXFLAGS_ALLOW='-ansi|-finline-functions'
For the second version:
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -stdlib=libc++
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -fvisibility=hidden
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -fvisibility-inlines-hidden
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -ftemplate-depth-300

CGO_CXXFLAGS_ALLOW='-stdlib=.*|-fvisibility=hidden|-fvisibility-inlines-hidden|-ftemplate-depth[=-]\d+
-stdlib is already included in master, but the other options are missing. Note that -ftemplate-depth-300 uses - instead of =.

More options to add to the whitelists:
From #23094:
LDPATH: -Wl,-rpath=$ORIGIN/lib

From #23090:
LDPATH: -mwindows

From #23923:
CXXFLAGS: -ffat-lto-objects
CXXFLAGS: -fuse-linker-plugin

(Note that -fuse-linker-plugin is safe as long as -B is not used, and -B is never safe anyhow.)
From #23749:
LDFLAGS: -mmacosx-version-min=10.8

(We currently accept -mmacos-x for the compiler, but not the linker.)

From #23749:
LDFLAGS: -O3

From #23938:
CFLAGS: -w

(You meant to say 23904 instead of 23094 there; in case that bug accidentally gets marked as fixed by this fix.)

Thanks; I updated the comment.

Reported on #23749:
LDFLAGS: -shared

(Oops, missed it: likewise for 23090. Not sure what the right issue number there is.)
Seconding -mmacosx in the linker here; was just about to report that myself. I will point out that the macOS linker will complain if the values of this flag conflict for any object file; what does Go itself do?

-mwindows is from #23909

Corrected issue reference in comment above.  Thanks.
Go itself does not even know about the -maccosx option, and as such will not complain about it.

No; I meant to ask what does Go set in the Mach-O headers for the minimum version. I can make that a separate discussion, though.

@andlabs Currently when using internal linking the program requests OS X version 10.7.0.  When using external linking it's set by the system linker.  Let's take any followup on this to golang-nuts.

From #23749:
LDFLAGS: -Wl,-static

@andlabs In 1.10 the error message points to https://golang.org/s/invalidflag, and in 1.9.5 it will as well.

From  #24113:
CFLAGS: -arch

From #23749:
CFLAGS: -m32
LDFLAGS: -O3
LDFLAGS: -static-libgcc
LDFLAGS: -static-libstdc++

From #23749:
LDFLAGS:  -Wl,-z,relro

LDFLAGS: -Wl,-z,relro
Is from #24124 and I see a bunch more flags in the sample code.
CXXFLAGS: -std=c++11 - Already listed on this issue
LDFLAGS: -Wl,-undefined
LDFLAGS: -Wl,dynamic_lookup
LDFLAGS: -Wl,-unresolved-symbols=ignore-all
LDFLAGS: -Wl,-z,noexecstack
LDFLAGS: -Wl,-dn
LDFLAGS: -Wl,-dy

Might be a good idea to ask the user to use CGO_CFLAGS_ALLOW and friends to find out if there are more flags not in the sample code.

From #23749:
LDFLAGS: -Wl,--subsystem,windows
CFLAGS: -mwindows

From #23749:
-isysroot  (ios)
-mios-simulator-version-min= (ios)
-miphoneos-version-min= (ios)
-target  (for android)
-sysroot  (for android)
@eliasnaur Are these compiler options or linker options?

-target and --sysroot are put in CFLAGS, CPPFLAGS and LDFLAGS by gomobile on android. Note that I spelled --sysroot with one dash in #23749. Sorry.
-mios-simulator-version-min=, -miphoneos-version-min=, and -isysroot are passed to CFLAGS and LDFLAGS by gomobile on ios.

(You should probably put those iOS CFLAGS into CXXFLAGS as well, lest someone using Objective-C++ be blocked.)

I found another one: "-arch ". It's added to all of CFLAG, CPPFLAG, and LDFLAG and isn't the same as -march.

-Wa,-mbig-obj to link big objects.

From https://github.com/golang/go/issues/23749#issuecomment-372103938
-Wl,-undefined,dynamic_lookup

A couple more CFLAGS that I ran into in my current project:
-fsigned-char
-funsigned-char

LDFLAGS: -Wl,-sectcreate,segname,sectname,filename to add sections to the binary.
From a security standpoint, on Darwin this is also an important flag for adding the __RESTRICT,__restrict segment to prevent DYLD environment variable injection.

@JakeDOD what is this __RESTRICT,__restrict injection thing, out of curiosity?

@andlabs More generally, it allows a macOS binary to opt-in as a restricted binary and tell DYLD to prune dangerous environment variables like DYLD_INSERT_LIBRARIES, override the use of @rpaths, and limit Framework and dylib loading to the SIP protected locations /System/Library/Frameworks and /usr/lib respectively. See dyld.cpp source

In case they're missed:
From https://github.com/golang/go/issues/23749#issuecomment-374940745
CFLAGS: -fmessage-length=152
CFLAGS: -fdiagnostics-show-note-include-stack
CFLAGS: -fmacro-backtrace-limit=0

From https://github.com/golang/go/issues/23749#issuecomment-375931680
(Assuming CFLAGS): -stdlib=libstdc++

Reopening for 1.10.1 and 1.9.5 cherry picks.

CL 102818 OK for Go 1.10.1

authors: AlexRouSg JakeDOD andlabs andybons coolgeek6667 eliasnaur ianlancetaylor mattn olt


text:
duplicate import path items misdirects doc tool

What version of Go are you using (go version)?
1.10.2
What operating system and processor architecture are you using (go env)?
linux/amd64
What did you do?
go get github.com/go-gl/gl/v3.3-core/gl
go doc gl
What did you expect to see?
documentation for gl package
What did you see instead?
doc: no buildable Go source files in ../go/src/github.com/go-gl/gl
exit status 1
(notice path truncated)
temp fixed by renaming /go/src/github.com/go-gl/gl -> /go/src/github.com/go-gl/GL

If you just call go doc gl there would be no way for it to know which gl package you meant so it just selects the first found "package" and that so happens to be github.com/go-gl/gl which contains no source files.
So the proper way to use go doc would be to use the full import path go doc github.com/go-gl/gl/v3.3-core/gl

understood, and this isn't hugely important, but i do think go's tooling is one of its bright points, so..
"github.com/go-gl/gl" isn't even a package, there is only one package 'gl' installed, doesn't seem that hard for it to just keep going until it found a package, or, seems like a tree search leaf first would work, mostly.
also; if i already had a package 'gl' which was reporting documentation with 'gl', installing the above package, as a side-effect, breaks that, that's surprising to me, i would have hoped package installation was completely isolated. in some sort of shared development environment i might not be aware of other installs until it breaks.

Seems like it should be a fairly simple fix: if the directory suffix matches but the path can't be imported, keep searching.
Some starting points, if you'd like to give it a try:
https://github.com/golang/go/blob/5776bd558fd0cfe40c71dac549db148ea16c0fce/src/cmd/doc/main.go#L242-L247
https://github.com/golang/go/blob/5776bd558fd0cfe40c71dac549db148ea16c0fce/src/cmd/doc/main.go#L341-L357

authors: AlexRouSg bcmills splace


text:
x/build/cmd/gopherbot: document gopherbot actions and commands

As the @gopherbot skills grow more interactive, we need somewhere to document how the commands work.

A wiki page would be good.
/cc @dmitshur

We have one! https://github.com/golang/go/wiki/gopherbot
Might be worth backfilling the automated operations as well, but it has the interactive ones.

I was looking for Gopherbot's documentation today, and I clicked on its GitHub profile but I completely missed that the profile had a link to the wiki. Maybe it would be a little more helpful if Gopherbot had a profile that said something like "I help organize Go project issues. Visit my wiki link to learn how I can assist you."

@gopherbot's profile now has a link to its wiki page (https://golang.org/wiki/gopherbot).
I'll close this because everything that was mentioned here seems done, but feel free to reopen or make a new issue if there's more to do.

authors: FiloSottile bradfitz dmitshur mark-rushakoff


text:
Modify go get -u semantics for modules

What version of Go are you using (go version)?
1.11
What operating system and processor architecture are you using (go env)?
darwin/amd64
When running go get -u <module>, it seems that the semantic should be "update this one module".  However, as it is currently documented, it will not only update that module, but also fetch the latest updates from all dependent modules.
To update just a single module, I can run go get <module>, but here I believe the intuitive semantic is "go get this module if you don't already have it."
Proposal:  add a -R flag to indicate a recursive module update, so that:

go get <module> only downloads the latest module if the module is currently not present.
go get -u <module>  will fetch the latest version of the module according to the rules in go.mod
go get -R <module> will fetch the latest version of the module and also fetch the latest dependencies as well.

go get -u <pkg> has meant “upgrade <pkg> and its transitive dependencies” since long before modules. We aren't changing that for module mode: the idea is to extend the existing go get commands to work with modules, not to replace them with an entirely different set.
go get <module>@latest already means what you propose for go get -u <module>.
go get -m -u <module> already means what you propose for go get -R <module>.
If you only want to get a module if no version of it is already active, you can express that intent explicitly: go list -m <module> || go get <module>. (In most cases, though, it's simpler to let go build, go test, or go mod tidy fetch missing module dependencies rather than adding them explicitly yourself.)
At the moment there is a bit of ambiguity as to whether “transitive dependencies” should apply to packages or to modules in the absence of the -m flag (#26902), but in most cases that does not significantly change the outcome.
See also #28156 and #27643.
(CC @rsc)


text:
crypto/tls: TLS session resumption re-verifies the client's certificate chain

What version of Go are you using (go version)?

$ go version
1.12.x

Does this issue reproduce with the latest release?
Yes
What did you do?
When using TLS session resumption with client certificates, the server reverifies the client's certificate chain.
What did you expect to see?
When using TLS session resumption the server sends an encrypted (and MACed) ticket to the client, which the client presents during the next handshake. According to the spec, the session ticket is an opaque blob. The Go implementation encodes the client's certificate chain into the session ticket. If the server is able to authenticate and decrypt the session ticket presented by the client, this should serve as proof that the server communicated with this client before, and allow it to skip the (potentially computationally expensive) verification of the certificate chain.
Re-verifying leads to unexpected consequences if the tls.Config.VerifyPeerCertificate callback was changed such that it rejects the certificate in between the original and the resumed connection . Since the server re-verifies the certificate chain, it will reject the connection. The client doesn't perform any verification (other than checking that the certificate didn't expire) and is happy to resume a connection even if its VerifyPeerCertificate callback would have rejected the server's certificate chain on a new connection.

I noticed this while implementing TLS 1.3, and it's not an easy decision. The discrepancy between client and server is bad. On the one hand, if the verification policy has changed, the application probably expects it to apply to all connections, so we should re-verify. On the other hand, verification is expensive and session tickets are a performance measure.
Usually, it's servers that have the most performance concern, though, so I am tempted to resolve the discrepancy towards safety, by making clients reverify as well. Opinions?
(Related to #25352)

Since we're encoding the whole certificate chain in the session ticket, there's no point whatsoever to use session tickets with TLS 1.3 at all: The amount of data transferred during the handshake is (modulo some framing overhead) exactly the same as during a normal handshake, the computational cost of re-verifying the certificate chain is the same, and we're not even saving any roundtrips.
Couldn't one argue that the security properties of a resumed session should be the same as those of the original session? If the session was kept alive (instead of being resumed), the new policy wouldn't apply to that session either, right?

Landing here to +1 what @marten-seemann said. We've hit that exact issue while debugging slowness in Go servers using TLS 1.3 and ticket resumption. There's at least 140 KB of heap allocation on every handshake, largely due to parsing the certificate chain from the ticket.
We started trying to patch crypto/tls and make ticket generation and validation configurable, by adding the following to tls.Config:
GenerateClientTicket func(Certificate) (ticket []byte, err error)
ValidateClientTicket func(ticket []byte) error

We want to preserve the original behaviour, and only use custom tickets when these functions are provided by the user.
This needs a lot of testing still, and we're far from having anything concrete but I'd like to ask @FiloSottile if this is in the team's radar, and if what I describe above is potentially an acceptable solution to the problem.

Found what's almost certainly a bug related to this while reviewing CL 229122: if the mode is RequestClientCerts or NoClientCerts and the client doesn't provide certificates, VerifyPeerCertificate is not called on a new connection but is called on a resumed connection.

@FiloSottile what do you expect the behavior to be here for the VerifyPeerCertificate bug? (side note: it looks like the call is only gated by ClientAuth, if the client doesn't send anything you still get the callback)
It seems reasonable to me that if the server didn't request client auth that VerifyPeerCertificates wouldn't be called, even if the client did provide certificates (because the certificate processing code shouldn't be called) which matches what happens in new connections, but not resumed ones. Is the suggested behavior making resumed connections match new connections?

This is the current state of chain verification:

New connection
Resumed connection

Server certificates
Verified
Not verified

Client certificates
Verified*
Verified*

(*) For both TLS 1.2 and 1.3 the current behavior for client certificate verification is:

For new connections: client certificates are processed if ClientAuth >= RequestClientCert. VerifyPeerCertificates is always called if the number of certificates sent satisfies the ClientAuth level, including when ClientAuth == RequestClientCert and no certificates were sent.
For resumed connections: client certificates are always processed, regardless of ClientAuth level and if any certificates were sent. VerifyPeerCertificates is always called if the number of certificates sent satisfies the ClientAuth level, including when ClientAuth <= RequestClientCert and no certificates were sent.

There are two separate issues here to address:

server certificate and client certificate verification should be made consistent for resumed connections (either both reverifying or both skipping verification).
client certificate processing should be made consistent between new and resumed connections.

The performance/security trade-offs of the first issue have already been discussed. The second issue (which should probably be spun off into its own issue at this point) seems slightly clearer, although only applies if we keep reverification during resumption. During resumption we probably want to match the behavior of new connections, i.e. only process client certificates if ClientAuth >= RequestClientCert. This shouldn't degrade security of existing applications as processing/VerifyPeerCertificate would only be skipped when the level of ClientAuth was reduced since the initial connection. There is an additional question of if we want to continue calling VerifyPeerCertificates when ClientAuth == RequestClientCert and no certificates were sent, but I think that is less obvious.

Howdy @marten-seemann, @fiorix, @rolandshoemaker, and @FiloSottile, given that this issue needs some more thoughts and work, plus processing. Shall we perhaps punt it to Go1.17? Thank you, and happy holidays!

Let us share the crypto/tls patch we’ve been using since Go 1.14, only for TLS 1.3, to allow configuring a custom session ticket generator and validator. This is currently only used for internal traffic, no internet facing service is using this code.
The patch is experimental, and only serves for what is mentioned above. The idea is to add GenerateClientTicket and ValidateClientTicket to tls.Config for servers, and then adjust the server-side handshake of TLS 1.3 to use the custom code path when the Generator and Validator are set.
How did we get to this? A while ago @opsengine was profiling a Go service that was replacing a C++ service and noticed how the TLS handshake of the Go server was expensive. During the investigation we (mostly @opsengine and @
slasher-) found that the crypto/tls session resumption code was using the client cert for ticket and parsing it on every handshake, including when resuming connections. We wanted to use a custom ticket and realised we couldn’t do so unless we patched the crypto/tls package, hence the patch we’re sharing here: https://pastebin.com/B7pVXHwj

@fiorix @slasher- the patch has been battle tested in production for quite a while, should we start a PR?
@FiloSottile to give you more context, the change was inspired by fizz, which does something similar with tickets.
The main goal is to have the option of reducing CPU utilization for those servers with a high number of handshakes per second. This would be an "advanced" feature for users who supposedly know what they are doing. The default behavior would remain unchanged.
There are two reasons why having a custom ticket format helps performance:

client certificates are parsed and verified only once per session, across multiple connections
fewer heap objects allocated as a result of less ASN.1 parsing, which saves a non-negligible amount of work for the garbage collector

CC @golang/security
Looks like this didn't make 1.19.  Moving to backlog.  Please recategorize as appropriate.

authors: FiloSottile fiorix ianlancetaylor marten-seemann odeke-em opsengine rolandshoemaker


text:
net: cannot bind network card to send data on macos

go1.11.0
macOS Mojave
I added two default routes that direct all traffic to a single virtual network card. Then I want to send the test program data directly from the physical network card. I first used bind()，but an error occured while establishing the connection: [connect: no route to host]
On Windows and Linux, I can use the bind() function to bind network cards to send data. Is there any way to do it on a MAC?
var netAddr net.Addr

netAddr = &net.TCPAddr{}
netAddr.(*net.TCPAddr).IP = net.ParseIP("xxx.xxx.xxx.xxx")
dialer := net.Dialer{LocalAddr:netAddr}
conn, err := dialer.Dial("tcp", "yyy.yyy.yyy.yyy:mm")
if err != nil {
	log.Println(err)
	return
}
defer conn.Close()
log.Println("conn successful")

CC @mikioh

Is there a temporary solution?

authors: ianlancetaylor weiyixuan


text:
x/tools/go/packages: panic in go/types during (*loader).refine

What version of Go are you using (go version)?

example.com$ gotip version
go version devel +3b92f36d Thu Aug 29 08:57:06 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
go env Output
example.com$ gotip env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/bcmills/.cache/go-build"
GOENV="/usr/local/google/home/bcmills/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/tmp/tmp.b6FwBbMhR2/_gopath"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/bcmills/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64"
GCCGO="/usr/local/google/home/bcmills/bin/gccgo"
AR="ar"
CC="gcc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.b6FwBbMhR2/example.com/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build142029212=/tmp/go-build -gno-record-gcc-switches"

What did you do?

example.com$ gotip mod init example.com
go: creating new go.mod: module example.com

example.com$ gotip version
go version devel +3b92f36d Thu Aug 29 08:57:06 2019 +0000 linux/amd64

example.com$ gotip get -d golang.org/x/exp/apidiff@ec7cb31e5a562f5e9e31b300128d2f530f55d127
go: finding golang.org ec7cb31e5a562f5e9e31b300128d2f530f55d127
go: finding golang.org/x ec7cb31e5a562f5e9e31b300128d2f530f55d127
go: finding golang.org/x/exp ec7cb31e5a562f5e9e31b300128d2f530f55d127
go: finding golang.org/x/exp/apidiff ec7cb31e5a562f5e9e31b300128d2f530f55d127
go: downloading golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56
go: extracting golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56

example.com$ GOTRACEBACK=all gotip test golang.org/x/exp/apidiff

What did you expect to see?
example.com$ GOTRACEBACK=all gotip test golang.org/x/exp/apidiff
ok      golang.org/x/exp/apidiff        0.431s

(Or, a useful test failure output).
What did you see instead?
A panic, with a large backtrace involving golang.org/x/tools/go/packages and go/types
example.com$ GOTRACEBACK=all gotip test golang.org/x/exp/apidiff
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5cbf3b]

goroutine 11 [running]:
go/types.(*Checker).handleBailout(0xc0001205a0, 0xc000049c90)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/check.go:241 +0x98
panic(0x6d23c0, 0x961580)
        /usr/local/google/home/bcmills/sdk/gotip/src/runtime/panic.go:679 +0x1b2
go/types.(*StdSizes).Sizeof(0x0, 0x791540, 0x962280, 0x72a153)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/sizes.go:154 +0x1bb
go/types.(*Config).sizeof(0xc000175b80, 0x791540, 0x962280, 0xc00017c001)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/sizes.go:251 +0x51
go/types.representableConst(0x795ac0, 0xc00017c0d8, 0xc0001205a0, 0x962280, 0xc0001a2068, 0x40)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/expr.go:216 +0x9a6
go/types.(*Checker).representable(0xc0001205a0, 0xc0001a2040, 0x962280)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/expr.go:335 +0x68
go/types.(*Checker).convertUntyped(0xc0001205a0, 0xc0001a2040, 0x791540, 0x962280)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/expr.go:517 +0x898
go/types.(*Checker).assignment(0xc0001205a0, 0xc0001a2040, 0x791540, 0x962280, 0x72f02d, 0x14)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/assignments.go:46 +0x14c
go/types.(*Checker).initConst(0xc0001205a0, 0xc000059140, 0xc0001a2040)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/assignments.go:93 +0x25b
go/types.(*Checker).constDecl(0xc0001205a0, 0xc000059140, 0x794a80, 0xc00000dcc0, 0x794580, 0xc00000dce0)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/decl.go:362 +0x11d
go/types.(*Checker).objDecl(0xc0001205a0, 0x799c80, 0xc000059140, 0x0)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/decl.go:201 +0x754
go/types.(*Checker).packageObjects(0xc0001205a0)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/resolver.go:589 +0x38a
go/types.(*Checker).checkFiles(0xc0001205a0, 0xc00000e220, 0x1, 0x1, 0x0, 0x0)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/check.go:255 +0xbb
go/types.(*Checker).Files(...)
        /usr/local/google/home/bcmills/sdk/gotip/src/go/types/check.go:246
golang.org/x/tools/go/packages.(*loader).loadPackage(0xc000118000, 0xc00000d400)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:716 +0x555
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:574 +0x17f
sync.(*Once).doSlow(0xc00000d410, 0xc00003af80)
        /usr/local/google/home/bcmills/sdk/gotip/src/sync/once.go:66 +0xe3
sync.(*Once).Do(...)
        /usr/local/google/home/bcmills/sdk/gotip/src/sync/once.go:57
golang.org/x/tools/go/packages.(*loader).loadRecursive(0xc000118000, 0xc00000d400)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:561 +0x78
golang.org/x/tools/go/packages.(*loader).refine.func2(0xc000118000, 0xc000015790, 0xc00000d400)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:542 +0x35
created by golang.org/x/tools/go/packages.(*loader).refine
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:541 +0x73c

goroutine 1 [chan receive]:
testing.(*T).Run(0xc000100300, 0x72be57, 0xb, 0x73bae8, 0x482bd6)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:961 +0x377
testing.runTests.func1(0xc000100200)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1202 +0x78
testing.tRunner(0xc000100200, 0xc000075dc0)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:909 +0xc9
testing.runTests(0xc00000c180, 0x962700, 0x2, 0x2, 0x0)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1200 +0x2a7
testing.(*M).Run(0xc0000fe180, 0x0)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1117 +0x176
main.main()
        _testmain.go:46 +0x135

goroutine 6 [semacquire]:
sync.runtime_Semacquire(0xc000015798)
        /usr/local/google/home/bcmills/sdk/gotip/src/runtime/sema.go:56 +0x42
sync.(*WaitGroup).Wait(0xc000015790)
        /usr/local/google/home/bcmills/sdk/gotip/src/sync/waitgroup.go:130 +0x64
golang.org/x/tools/go/packages.(*loader).refine(0xc000118000, 0xc000076a80, 0x1, 0x4, 0xc00000d240, 0x1, 0x4, 0xc000082ca8, 0x40c768, 0x10, ...)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:546 +0x767
golang.org/x/tools/go/packages.Load(0xc000082d58, 0xc000114600, 0x1, 0x1, 0x2d, 0xc000116800, 0x39, 0x80, 0xc000082d38)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/tools@v0.0.0-20190312151545-0bb0c0a6e846/go/packages/packages.go:178 +0xf0
golang.org/x/exp/apidiff.load(0x72bec5, 0xb, 0xc000016300, 0x1d, 0x4c, 0x80, 0xc000104400)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/exp@v0.0.0-20190731235908-ec7cb31e5a56/apidiff/apidiff_test.go:124 +0x109
golang.org/x/exp/apidiff.TestChanges(0xc000100300)
        /tmp/tmp.b6FwBbMhR2/_gopath/pkg/mod/golang.org/x/exp@v0.0.0-20190731235908-ec7cb31e5a56/apidiff/apidiff_test.go:29 +0x2a8
testing.tRunner(0xc000100300, 0x73bae8)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:909 +0xc9
created by testing.(*T).Run
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:960 +0x350
FAIL    golang.org/x/exp/apidiff        0.144s
FAIL

CC @matloob @ianthehat @jba

I'm not sure whether the bug here is in go/types or in its particular use in go/packages.

Hmm, looks like the problem may have been an old x/tools dependency in x/exp/go.mod. After updating a few versions of things it no longer reproduces for me.