text:
Godot crashes on MacOS M1 when opening certain scenes with Compatibility renderer

Godot version
4.1.2.stable
System information
Godot v4.1.2.stable - macOS 14.0.0 - Vulkan (Forward+) - integrated Apple M1 - Apple M1 (8 Threads)
Issue description
I've had random crashes here and there on 4.1.1-4.1.2 but one that I encountered consistently last night is the entire editor crashing upon opening a certain scene file.
Neither the scene nor its dependencies are corrupt in any way - I've successfully opened the same scene and project on my Windows and Linux machines.
After trying to find similar cases online, I stumbled upon this forum post where the answer mentions switching away from Compatibility mode to solve the issue.
I tried this and I am now successfully able to open the scene, either in Mobile or Forward+ mode. Still, opening the scene in Compatibility mode will always crash Godot. Even when its tab is left open, Godot just crashes on startup now.
Apple Problem Report: https://pastebin.com/eaUz70Au
Steps to reproduce

Open reproduction project on a MacOS M1 machine
Make sure renderer is set to Compatibility mode
Open item_drop.tscn

Crash should occur afterwards.
Minimal reproduction project
repro_1010.zip

Thanks for the report! Consolidating in https://github.com/godotengine/godot/issues/72469.

authors: Calinou wayjack


text:
Saving layout does not save state without distractions

Tested versions
v4.3.rc1.official [e343dbbcc]
System information
w10 64
Issue description
Whenever I start godot I have to activate "distraction-free mode" in the scprit editor, godot does not save that when the layout is saved.
I have the "separate distraction mode" option activated.
Steps to reproduce
Activate the "separate distraction mode" editor option.
Turn on distraction-free mode in the script editor
Close godot
open godot
Check that the script editor has distraction-free mode disabled again.
You will have to activate it again.
Minimal reproduction project (MRP)
...

See your previous report on the same topic and this comment https://github.com/godotengine/godot/issues/56237#issuecomment-1000996117

It's true, I forget after a while, I think I should use the open bug finder more before creating one.

Easier to find if you open the list of your own issues (it's the circle with a dot in the top right corner of the page if you're on desktop)

source: gharchive/issue

authors: AThousandShips Torguen

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Quick open search doesn't always prioritize shorter results

Tested versions
v4.4.dev.custom_build [db66bd35a]
System information
Godot v4.4.dev (db66bd35a) - macOS 14.5.0 - Multi-window, 1 monitor - Metal (Forward+) - integrated Apple M1 Max (Apple7) - Apple M1 Max (10 threads)
Issue description
It will only show those on top if it's an exact match ("player_controller"). The results are also shown randomly and not sorted alphabetically which makes it a bit harder to navigate

Steps to reproduce
N/A
Minimal reproduction project (MRP)
N/A

Fixed in master by https://github.com/godotengine/godot/pull/98278 I assume


text:
Scene Tree Dock: added ability to deselect items when clicking on empty space

This is how it looks in action: https://i.imgur.com/tMECINu.gifv

Dont' know why these three - https://i.imgur.com/diY2pJj.png - are here, rebasing didn't helped.
Let me know if this is mergeable in current state (because it shows me correct that just 3 files changed - https://i.imgur.com/YZ9bce3.png, so I assume that another 3 commits will not be counted in PR merge).
Or if need to, I'll just recreate PR.

source: gharchive/pull-request


text:
Empowered modulate of tiles in TileSets, and modulate of item in listitem

Hey guys,
I've made this because, when trying to generate colored TileSet resources dynamically by modulating different pieces of a base black and white tilesheet, I felt the modulation of tiles was not supported by the editor as well as it could be.

tile_set_modulate and tile_get_modulate existed, but they weren't exposed to the user. This was pretty weird, but maybe I'm the first guy this desperate to look for it?
in the tile_map_editor_plugin, the tile previews weren't colored using their modulate color. This in part because modulating the color of an Item in ListItem wasn't supported.
in the editor preview of drawing a tile, the modulate also wasn't shown.

All the modulate values in the draw functions used in these used a dummy white color, with alpha 0.5. It was a cinch to add a modulate color to Item and use that instead of the dummy white color. The default value of the Item struct is still the white color, so it is backward compatible.
See the screenshot for how it looks for me. All the colors are modulated in the tileset.

Thanks for reading!

I am fine with this function, although I think we should probably also support color modulation per tilemap tiles and painting them as an extension to this too.
Needs to be rebased, though.

OMG. I totally forgot about this PR and implemented a similar fix when I ran into this issue when fixing another unrelated issue. In #17443 it is ItemList::set_item_icon_modulate. But if I understand correctly, my PR and this PR only partially overlap so this could still be merge after conflict resolution. Sorry about that.

Hey guys! I resolved the conflicts.
Are there other things I should do?
Code-wise, I'm thinking of renaming the functions ending in _modulate to _modulate_color.
Git-wise, I should probably clean this up in its own branch and make a new PR?

I'll get on this tonight when home from work :smile:

Code-wise, I'm thinking of renaming the functions ending in _modulate to _modulate_color.

I think it's fine as is, we also use just modulate for the CanvasItem property which modulates the color.

Git-wise, I should probably clean this up in its own branch and make a new PR?

You can clean it locally and force push to your remote master branch to update this PR. Here you'd likely just need to run something like this:
git remote add upstream https://github.com/godotengine/godot
git fetch upstream
git rebase -i upstream master

I tried to rebase the commit in @Willowblade's branch (since GitHub allows maintainer to edit PR branches) but apparently messed up, since I pushed a "clean" upstream/master which closed this PR. Will push the commit manually.

Actually this was implemented already in the master branch with #20391.

I'll add the editor plugin changes which are not part of #20391 though.

Done in 4d6a6e081.

authors: Noshyaar Willowblade akien-mga reduz


text:
filename is retained when enteringa folder manually and yesno dialog to create dialog if can_create_dir #22509

And no tabs.

Hi, thanks for your contribution.
Could you please move the fixed issues in your PR comment instead of its title ? This helps sorting as Github automatically close them if you put "closes" of "fixes" in front of it. :)

authors: groud spongeboburu


text:
GUI improvements in the asset library editor.

This is my first pull request. It does 2 things in the asset library popup.

Splits the description section from the preview section.
from this:

to this:

It centralizes the thumbnail overlay for video-thumbnails.
from this:

to this:

Thanks!

authors: akien-mga gdupr


text:
Document AnimationPlayer's quirks in regards to late updates

Just what it says on the tin.
Thanks to nobuyuki for alerting me to this by asking about a glitchy-looking behavior caused because they didn't know about it.
Do note: It's possible that the solution to this might not be to update the documentation but instead be to just update immediately on play and play_backwards.
If poked I'll make a PR, but it's a 2-line change.

Documentation changes look fine, could you squash commits together?

Do note: It's possible that the solution to this might not be to update the documentation but instead be to just update immediately on play and play_backwards.
If poked I'll make a PR, but it's a 2-line change.

Might be worth having a PR for review. For 3.2 I'd likely stick to the doc changes to avoid breaking compat, but we can review the proposed changes for 4.0.

Done.

Thanks!


text:
Increase the size of the item list editor popup

This makes it easier to edit large amounts of items.
This partially addresses https://github.com/godotengine/godot-proposals/issues/368.
Preview

Thanks!

authors: Calinou akien-mga


text:
Fix JNI emitSignal with one or more parameters (3.2)

Cherry-pick of https://github.com/godotengine/godot/pull/39028 for 3.2

Related issue: https://github.com/godotengine/godot/pull/39028

Replaced by #39047.
@timoschwarzer Thanks for highlighting the issue.
While this PR provides a fix to the issue, it unnecessarily limits the number of parameters that can be passed for a signal to VARIANT_ARG_MAX.

@m4gr3d thanks :) This should be changed in the JNI method for GodotLib.calldeferred, too, shouldn't it?

Edit: Just saw that Object::_call_deferred_bind is protected.

yep :) that prevents us from using a similar logic.


text:
Fix specular render_mode for visual shaders

Seems like "disabled" specular render mode should be first in order to correct processing, this may change the specular modes for existed visual shaders, but this will fix schlick_ggx specular mode...
Fix https://github.com/godotengine/godot/issues/41525
PS. I think documentation is wrong and the default specular mode is "specular_disabled"

I think documentation is wrong and the default shader mode is "specular_disabled"

I am worried that there are other places that rely on the order of render modes to work properly.
Plus this change just makes it so that specular_mode_disabled can't be used.

Alright - I did it another way(less elegant):

@clayjohn recheck again

Plus this change just makes it so that specular_mode_disabled can't be used.

As I noted - It's used by default and can be omitted

Thanks!

authors: Chaosus akien-mga clayjohn


text:
X11: Include limits.h for LONG_MAX

Fixes #44030.
I can't reproduce it myself on Mageia Cauldron with latest toolchains, but this should fix it.

Cherry-picked for 3.2.4.


text:
Update the initialization timeline for the Godot fragment

This is done to be more consistent with the previous activity based timeline as the discrepancy can cause initialization timing errors.
It also facilitates migration to the fragment based architecture for projects directly integrating the Godot Android library.

Thanks!

Cherry-picked for 3.2.4.


text:
Improve compiler version extraction

With this pr the compiler version checking improved in a way where only the major version is mandatory and along with the major field we can extract minor, patch, two alphanumeric fields and a date, so more sophisticated version checks are possible. The accepted string is 1.2.3-abc+def 20210531 where any field can be omitted except the major version. I hope this enough to cover the version numbers for a long time.
I have tested it with many distro's compiler version strings (Debian, Ubuntu, Arch, Fedora) but I may have missed something, so please have a look at the pattern here.
Fixes: #48815

Thanks!


text:
Exclude atomic lib under FreeBSD using LLVM

Exclude atomic lib under FreeBSD using LLVM
FreeBSD doesn't implement 64 bit atomic functions for i386, and LLVM doesn't provide atomic builtins as GCC does, thus compilation fails.
Removing linking with atomic libs if using LLVM under FreeBSD solves the problem.
Fixes #54393

Thanks! This should likely be changed for the master branch too.

authors: akien-mga robfram


text:
Rename TouchScreenButton's textures

Part of #38937
New names taken from TextureButton.

Closing for the same reason as #54430

Well I think it's still worth fixing until it's actually removed.

Ok, doesn't look like it's going to be removed anytime soon.

Thanks!

authors: KoBeWi akien-mga


text:
Only print OpenGL 2D batching status when in verbose mode

Since the new asynchronous shader compilation message prints to a separate line (as it can be quite long), this frees up space in the editor Output panel.
See https://godotforums.org/discussion/comment/57977/#Comment_57977 for context.

Ah yes I'd been meaning to change this. :+1:
At the time I'd actually wanted to just print e.g. a "B" in the line above, e.g.:
OpenGL ES 3.0 Renderer (B): GeForce GT 730/PCIe/SSE2

The only reason I didn't do this was that the video driver message was printed way before the batching was initialized, and it didn't seem worth making this bit of code spaghetti just for a logging message.
But now most people are using it and it is default, it seems fine just to make it verbose.

Thanks!

authors: Calinou akien-mga lawnjelly


text:
Make TextEdit's scroll faster when the target line is far away

Previously, pressing Alt didn't make scrolling in the script editor faster, only brought the target line further. This is wrong when smooth scrolling is enabled.
Fixes #66601

Would be good to make the commit message more explicit (e.g. like what I did with the PR title).
I'm also not sure this is the correct fix. It would be good to check first why it's not working, because both smooth and non smooth scrolling use the same code path that calls:
                                } else if (mb->is_alt_pressed()) {
                                        // Scroll 5 times as fast as normal (like in Visual Studio Code).
                                        _scroll_up(15 * mb->get_factor());

And _scroll_up has logic to handle both smoothed and non smoothed scrolling. Maybe that logic is broken and needs to be fixed, but what this PR does seems to be a workaround.

@akien-mga Both smooth and non-smooth scrolling set a target line to scroll to, non-smooth scrolling goes to that target line immediately. Smooth scrolling sets the "scrolling" boolean under some conditions and then on the physics frame it executes the scrolling itself.
Since only the physics frame creates a vel variable to use for scrolling, changing the speed there is the least intrusive.
A different option would be to start scrolling faster if the target line is far away. Whaddya think?

I can't work out why checks are failing and how to make them stop failing.

Did you try rebasing on newest master?

I'm sorry, I messed up the rebase

I can't work out why checks are failing and how to make them stop failing.

The unit tests are failing, looks like the scroll position is mainly off by one.

Ah I think it's just a silly oversight :smile_cat:

Yee, it's all ready now. I think it was something about physics FPS being infinite in the test suite, or something... though it can be worked around with a slightly more complicated logic. Also I made it a little less subtle than in the picture above, it's gonna be quite noticeable even without using Alt+Scroll

Decided not to add any settings to TextEdit for this. This is a temporary hack to make Alt+Scroll work better, but the whole way scrolling is implemented will likely need to be rebuilt from the ground up eventually.

authors: KoBeWi MewPurPur Paulb23 akien-mga


text:
Fix tools GDScript export default value type not matching actual type

Partially fixes #66791 (only partially since there seems to be multiple issues)
This fixes the default value for a tools export being stored as the wrong type when the type annotation doesn't match the primitive type. E.g:
export var test: int = 3.5 // Float being set for int type
export var test2: float = 3 // Int being set for float type
export var test3: bool = 2 // Int being set for bool type

I checked out the 3.5 branch to see how it was handling this, which is by converting the default value before storing it:
/* 3.5 branch */

Variant::Type default_value_type = c->variables[i].default_value.get_type();
Variant::Type export_type = c->variables[i]._export.type;

// Convert the default value to the export type to avoid issues with the property editor and scene serialization.
// This is done only in the export side, the script itself will use the default value with no type change.
if (default_value_type != Variant::NIL && default_value_type != export_type) {
	Variant::CallError ce;
	const Variant *args = &c->variables[i].default_value;
	member_default_values_cache[c->variables[i].identifier] = Variant::construct(export_type, &args, 1, ce);
} else {
	member_default_values_cache[c->variables[i].identifier] = c->variables[i].default_value;
}

And so I just ported that over.
Testing notes
See #66791 for a reproduction project. The variations to test are int, float and bool (Are there any others?).
You'll also need to test around some of the other issues with tools scripts (mentioned in #66791). Reloading the scene a couple times should be enough

Fixed by #69416


text:
Update GraphEdit to automatically arrange nodes if nothing is selected

…lected.
https://github.com/godotengine/godot/pull/70066#issuecomment-1370303620

Rearrange is active, I did not know it only arranges selected nodes (and does nothing without message if no nodes are selected)

According to a random new developer, the ui expected behaviour is that the graph edit auto arranges if nothing is selected and the button is pressed.
Alternatively we can add a warning message, this fix feels more intuitive.

Thanks!

authors: akien-mga fire


text:
Implement "looped" signal for AudioStreamPlayer, 2D and 3D

Track the number of loops on the player and if it changes, emit a singal.
Closes godotengine/godot-proposals#1641

Ah, I misunderstood a bit. The looped count is something that already exists, and the user determines the upper limit, right? But I think there may be a cleaner way to fire signal.
Also I remember there was some problem with the AnimationPlayer looped flag and I didn't implement it. I'll take a look at it later...

It makes no sense to count the number of loops on the signal side. Also, it may cause an implicit overflow. Signal should only notify the fact that a loop has occurred and should be counted on the user script side.

The overflow here does not matter, even if it overflows the number still changes and this is what matters to fire the signal. I just used the type that get_loop_count() returns.

Moreover, the looped signal should fire only when a loop occurs, so should not fire at the same time as finished.

It only fires if the stream did not end, hence the else if on the condition that checks if it ended.

I oppose the merging of this PR on the following grounds:

Emitting a signal from the audio thread is wrong in many accounts 1) Audio thread must never run complex logic that can allocate memory or run game logic as a result of a signal, because its time (callback) constrained. 2) A signal emitted from the audio thread is useless and dangerous.
You can already query the loop count, so all you have to do is check every frame whether the audio has looped.

This PR originally tried to fire a signal by storing the loop counts of all streams in a map, is this the correct way to do it? Considering the many possibilities of not using the signal, it seemed to me to be expensive.
Perhaps the correct way to do it is to set up a script that monitors only certain Streams when a signal is needed on the user's side.

Perhaps the correct way to do it is to set up a script that monitors only certain Streams when a signal is needed on the user's side.
Is there a way to check if a signal is needed at all? And if so, how should it track the looping? If emitting a signal on the audio thread is not a good idea, maybe just using the old approach with the dictionary is fine, as long as I can somehow know if I need to track only certain streams.

authors: TokageItLab reduz vaartis


text:
Fix Unix temp file creations when using is_backup_save_enabled.

See comments in https://github.com/godotengine/godot/issues/74968.

Does this really fix the linked issue? It mentions 4.0.1.rc1 but the PR this is fixing up was merged after 4.0 and not cherrypicked.

Does this really fix the linked issue?

It's hard to say what was the crash reason of the issue (it might be massive descriptor leak, or something else), but comments are mostly related to the temp files.

For the reference, it is fixing the regression from #73156, mkstemp writes temp file path to its argument and opens the file, returning file descriptor.
So if (!mkstemp(path.utf8().ptrw())) was:

writing path to the temporary UTF-8 string, which is not saved.
leaking open file descriptor.

Now it's using the returned descriptor and converting file path back to String. fdopen do not duplicate the descriptor, so there's no need to close it if it was successful, but it should be closed and deleted if it fails.

@yudi-azvd said in #74968:

@YuriSizov I think you are correct. First I've built the master branch by mistake. I noticed it then built the 4.0 and it's the one I'm using now. Since then these extra files did not appeared.

So I don't think it fixes #74968.

Indeed it doesn't fix the original problem from the linked issue, just a side-comment that uncovered a problem with a recent PR in master.

@bruvzg I created the issue #75078 so you can tag your PR with it.

Thanks!

authors: YuriSizov adamscott akien-mga bruvzg


text:
Add NavigationRegion function to change navigation map

Adds NavigationRegion function to change navigation map.
Currently it is only possible to switch the navigation map of a region by using the NavigationServer API directly.
Having it on the node makes it work with enter / exit of Scenetree as well as making the possibility to switch maps more discoverable.
Build on top of https://github.com/godotengine/godot/pull/77175 that needs to be merged first.

Thanks!


text:
[graph editors] Keep port icons crisp at high zoom levels and remove artifacts

With zoom levels up to 207%, the port icons are noticeably stretched. You can also observe interpolation/sampling artifacts because the circle touches all edges of the SVG's viewport. This can be solved with size overrides and adjusting the circle/the image's dimensions.
Before/After:

TIL about ImageTexture.set_size_override. This method seems pretty useful and I'm surprised it's not exposed as a property, nor in the inspector for ImageTextures (it is for PortableCompressedTexture2D though).
PS: The class reference description for ImageTexture.set_size_override() is misleading, as it overrides the size the texture is rendered at (only affects 2D). It does not actually resize the image data.

Same as you, I just learned about that method today and wished I had sooner. (I stumbled upon it accidentally while looking for a way to implement this exact functionality)
I think we should definitely expose size_override as a property or increase its visibility in another way, at least through the documentation.
When even you, Calinou, didn't know about it, then we definitely need to take some action :)

Thanks!

authors: Calinou Geometror YuriSizov


text:
Make ShaderMaterial always the last option in inspector resource dropdowns

ShaderMaterial is the "advanced" option, while the "basic" options should be listed first for easier accessibility (and because they're generally used more often).
This makes sky and particle materials consistent with canvas/spatial/fog materials on this aspect.

This causes GDExtension compatibility errors, is this expected? I'm only changing the entries' order – no entry has been added or removed.
Error: Validate extension JSON: Error: Field 'classes/GPUParticles2D/properties/process_material': type changed value in new API, from "ShaderMaterial,ParticleProcessMaterial" to "ParticleProcessMaterial,ShaderMaterial".

Well anything that affects extension JSON is considered a compatibility breakage. You can just add it to the expected file, as I think it doesn't really break anything.

while the "basic" options should be listed first for easier accessibility (and because they're generally used more often).

Except for CanvasItems, where ShaderMaterial is used most of the time (from my experience).

Thanks!

authors: Calinou KoBeWi akien-mga


text:
Support SetContext method on *ghttp.Request to intergrate with opentelementry

1. What version of Go and system type/arch are you using?
go 1.14, linux/amd64
2. What version of GoFrame are you using?
v1.14.5
3. Can this issue be reproduced with the latest release?
yes
4. What did you do?
I want to use opentracing or opentelementry with goframe
5. What did you expect to see?
I want to implement a middleware intergrated with opentelementry, but context of ghttp.Request can not be assigned by generated new ctx, so when we try to begin a new tracing context like
ctx := r.Context() // r is *ghttp.Request
ctx = global.TextMapPropagator().Extract(ctx, &metadataSupplier{
        metadata: metadata,
    })

how can we pass the new context to following handler methods?
I hope, ghttp.Request  can provided a public method like
func (r *Request) SetContext(ctx context.Context) {
	r.context = ctx
}

If so, the tracing context can be passed to following handler methods
6. What did you see instead?
context of ghttp.Request can not be assigned directly, it's really hard to intergrate opentelementry with goframe, the context usage is different from others like in gRPC or other frameworks

@isword123 The ghttp.Request is inherited from net/http.Request, so you can use r.Request = r.WithContext(ctx) change its underlying conetxt. Thanks for your proposal, I'll add a new method r.SetCtx for convenience.

Thank you for your advice. I'll try with this method.

repo: gogf/gf


text:
Proxy for outgoing traffic when mirroring

I\m having some slight trouble finding any configurable setting for proxy when doing outgoing/external traffic from GOGS, for example when trying to mirror content from github.
I've tried setting http_proxy, https_proxy and no_proxy but it doesn't seem like it's getting picked up.
Any suggestion or is this considered a RFE?

Hmm... Gogs currently does not have this feature, what would be the use case of this?

Well, exactly as I mentioned in the first post. But I can explain a bit further.

Imagine a scenario with the following:
You have a internal server on which you deploy gogs. This is to be used as your internal git server and all is well and all until you realize:
"Hey, I want some content from github appearing in my gogs automagically"
At which point you discover that there is a nifty mirror feature (https://github.com/gogits/gogs/issues/63) and your day is saved. You quickly login to gogs, click on mirror and are met with a nasty error saying:
"Migration failed: Clone: exist status 128 - fatal: unable to access 'https://github.com/gogits/gogs.git/': Failed to connect to github.com port 443: Connection refused"
You suddenly remember that your internal gogs server is running behind a firewall which does not allow any external traffic. Everything has to be going through a http-proxy.

Yeah, you probably get the idea.
Normally this is solved by honoring the normal proxy environment variables http_proxy, https_proxy, no_proxy. And perhaps even adding local config parameters in the application, but at least following the environment variables.
See for example:
https://www.gnu.org/software/wget/manual/html_node/Proxies.html

Thanks for the great detail!

Normally this is solved by honoring the normal proxy environment variables http_proxy, https_proxy, no_proxy

Gogs currently calls git clone --mirror xxx for migration so I think the proxy you want to is this: http://stackoverflow.com/questions/783811/getting-git-to-work-with-a-proxy-server
Even if Gogs lets you set a config option, it still affects global.

ping @Rydekull

Close due to lack of feedback.
You can continue discussion on https://discuss.gogs.io/.

This actually seems to work in my setup. I did the following:
Added EnvironmentFile=/etc/default/gogs to my systemd service definition to have a portable way to define custom environment variables.
In that file I added:

Use proxy when mirroring
http_proxy="http://proxy.example.com:3128"
https_proxy="http://proxy.example.com:3128"
no_proxy="127.0.0.1,127.0.1.1,localhost,.example.com"

And restarted gogs:
systemctl restart gogs.service
And it worked. Not initially as expected though. git is kind of quirky in that it actually needs https_proxy defined for https repositories, where most software just uses http_proxy if https_proxy is not defined.
Additionaly i initially had https_proxy="${http_proxy}" in this file as environment variables is normally sourced in a shell that substitutes the variables, but gogs complained that it could not understand the url "${http_proxy}" so I made an explisit definition.

authors: Rydekull Unknwon hkbakke

repo: gogits/gogs


text:
No Email Duplication Check when create users from Admin Panel

Gogs version: 0.9.115.0103
Git version: Go1.5.4
Operating system: Ubuntu 16.04
Database:

[ ] PostgreSQL
[ ] MySQL
[x] SQLite

Can you reproduce the bug at https://try.gogs.io:

[ ] Yes (provide example URL)
[x] No (I do not have admin access to it.)
[ ] Not relevant

Log gist:

Description
Minor bug. I turned off the public registration. When I create a new user in Admin Panel, if I use an Email address which has already been used, I can still create the new user. (But it does complain in the Edit page later, which is good.)

Patch has pushed to fix this issue, please test on master branch.

Tested on docker image. Fixed!
Thx!

authors: RodenLuo Unknwon


text:
Simple html filter for repository description

For issue #4822

add new template filter function
use it in repo home template

How it looks:

Simple enough.
Ready for review.

😂 Is it overkill? Description is meant for at most one paragraph, otherwise it should be considered as "document".

I don't think it's overkill.
Sometimes we want to describe project more "structured", like:
This is my awesome project.
- php: 7.0
- engine: yii-1
- db: mongo

But now it can't even "emulate" MD - newlines will be stripped, all text will be merged in one long string.
If you have suggestions how to use here some minimal markup I'll rethink things.

If the main concern is the new line, then we could just unescape \ns just like links, no need to make it fully markdown-able.

Now it looks like that:

I like it :wink:

I think link and list are the only two things we should support in the repository description, anything beyond that should be put in a sophisticated document (i.e. README.md).

Well, I can guess how to do that.

Make new limited sanitizer with custom policy - allow only p, br, span, ul, ol, li
Make new filter which do str -> md -> sanitizer

It worth a new issue I think.

That's overthinking, what you have done on this PR right now is sufficient already.

That's overthinking, what you have done on this PR right now is sufficient already.

I mean if we will do limited markdown filter then it will be another PR.
Not this PR.

Thanks!


text:
部署优化

@liverpool8056

Add ngr_install.sh and ngrAdmin_install.sh to support quick install

repo: gogo-easy/ngr


text:
Can't manage private projects with the harbor_project resource

Describe the bug
When creating a project with "public = true" I receive an "Error: Resource not found /projects/123"
The project is actually created in harbor. I assume it fails when trying to get the info of the just-created project.
When I set the project to "public = true" it works just fine.
Same behavior when trying to import an existing harbor_project into the tf state:
It works if public and fails if private.
To Reproduce
Create project with "public = false" (default).
or
Import project which is already private.
Expected behavior
I would expect this to not fail.
Additional context
Example log:
harbor_project.main: Creating...
2023-07-20T15:44:15.536+0200 [INFO]  Starting apply for harbor_project.main
2023-07-20T15:44:15.536+0200 [DEBUG] harbor_project.main: applying the planned Create change
2023-07-20T15:44:15.537+0200 [INFO]  provider.terraform-provider-harbor_v3.9.4: 2023/07/20 15:44:15 [DEBUG] []: timestamp=2023-07-20T15:44:15.537+0200
2023-07-20T15:44:15.781+0200 [ERROR] provider.terraform-provider-harbor_v3.9.4: Response contains error diagnostic: tf_proto_version=5.3 tf_rpc=ApplyResourceChange @module=sdk.proto diagnostic_severity=ERROR diagnostic_detail= diagnostic_summary="Resource not found /projects/1136" tf_provider_addr=provider tf_req_id=88f28883-acf6-3564-5cbd-59f82c9dc3ed tf_resource_type=harbor_project @caller=github.com/hashicorp/terraform-plugin-go@v0.14.3/tfprotov5/internal/diag/diagnostics.go:55 timestamp=2023-07-20T15:44:15.781+0200
2023-07-20T15:44:15.784+0200 [DEBUG] State storage *statemgr.Filesystem declined to persist a state snapshot
2023-07-20T15:44:15.784+0200 [ERROR] vertex "harbor_project.main" error: Resource not found /projects/1136
│ Error: Resource not found /projects/1136
│
│   with harbor_project.main,
│   on main.tf line 38, in resource "harbor_project" "main":
│   38: resource "harbor_project" "main" {
not sure if this is an issue for the terraform provider or the main harbor project. I didn't try to reproduce it manually through the API.

Provider Version [3.9.4]
Terraform Version [1.5.3]
Harbor Version [v2.8.2]

Hello,
it looks like a lack of permissions with the account you use to create the project.
if you don't use an "admin" account it's probably like this issue : https://github.com/goharbor/terraform-provider-harbor/issues/328

Hi,
seems so. I just finished digging around a bit.
Actually a 401 unauthorized is returned from the API for those requests. Should the terraform error message be more descriptive of this? "Resource not found" is a bit vague.

indeed, we can improve it

should this simply handle the 401 error (which imho is the wrong error, it should be 403) or should it also hint towards this problem?

authors: flbla pidreher

repo: goharbor/terraform-provider-harbor


text:
docs: Remove /layouts/_default/single.html from home page template docs

Linux Mint 18, Hugo v0.21
According to "Homepage" docs Hugo will use the following prioritized list to render content/_index.md:

/layouts/index.html
/layouts/_default/list.html
/layouts/_default/single.html
...

But it ignores _default/single.htm. On Using _index.md:

The Home page, though technically a List page, can have its own template at layouts/index.html rather than _default/list.html. Many themes exploit this behaviour so you are likely to encounter this specific use case.

This is confusing. Looks like /layouts/_default/single.html is not supposed to be used for homepage rendering. I assume this is a mistake in docs.
I also created a repo to reproduce this issue.

Soon we will switch to a overhauled version of the docs. This issue has already been fixed in https://github.com/rdwatters/hugo-docs-concept/issues/117. Hence I consider this issue as fixed as well.

authors: digitalcraftsman rmuratov

repo: gohugoio/hugo


text:
Updated disqus template with configuration-variables

The universal code for disqus is as follows. The existing script template doesn't seems to be working proper.

Please enable JavaScript to view the comments powered by Disqus.

I have updated the script template matching with the universal code.
https://help.disqus.com/customer/en/portal/articles/2158629

I had the same problem. Using version v0.18.1 BuildDate: 2017-01-12T16:23:41-02:00. As a workaround I've used the "Conditional loading" template from http://gohugo.io/extras/comments/

@anthonyfok is this still relevant?

@anthonyfok is this still relevant?

Thanks for the heads up @ bep!
@sasivarnan, thank you so much for your contribution!
I am sorry that I had missed your Pull Request, and had instead saw @yihui's Pull Requests #3639 and #3655 from this past week, and had merged his PRs.  The fixes are very, very similar, so you have the credit of being here first!  :-)
I apologize for my oversight, and thank you again @sasivarnan!

Oops. I didn't notice there was an existing PR. Yeah, this one is very similar to the one I sent.

authors: anthonyfok bep sasivarnan tbrugz yihui


text:
Possible bug with non-go pruning

I've created a sample project to demonstrate this issue. The project has a Gopkg.toml file that clearly states not to prune "non-go" files for the specified package yet after the dep ensure some C files are missing thus go build does not work: https://github.com/tapir/dep-prune-test
To reproduce
go get -d github.com/tapir/dep-prune-test
cd $GOPATH/src/github.com/tapir/dep-prune-test
dep ensure
go build

What version of dep are you using (dep version)?
0.4.1
What dep command did you run?
dep ensure
go build
What did you expect to see?
Expected that the project built correctly
What did you see instead?
test/vendor/github.com/ethereum/go-ethereum/crypto/secp256k1
vendor/github.com/ethereum/go-ethereum/crypto/secp256k1/curve.go:43:10: fatal error: libsecp256k1/include/secp256k1.h: No such file or directory #include "libsecp256k1/include/secp256k1.h"

I came across the same issue and determined that for go-ethereum I had to specify:
[[prune.project]]
    name = "github.com/ethereum/go-ethereum"
    non-go = false
    unused-packages = false

specifically, setting unused-packages = false for pruning option for go-ethereum. There must be an edge case throwing off dep for pruning unused packages that use a C library

Sorry i didn't respond at the time, but yes, @pkieltyka has the expected resolution. Glad you folks got this sorted!

authors: pkieltyka sdboyer tapir

repo: golang/dep

license_source: bigquery


text:
Idea: Allow creation of aliases in Manifest.json

First of, thank you so much for your hard work on this project. Y'all are doing great work on this issue. Second of all, my apologies is this isn't the right place to open this issue.
My idea is to an introduce an alias keyword in manifest.json. For instance:
{
    "dependencies": {
        "github.com/pressly/chi": {
            "version": "^2.0.0",
            "alias": "chi"
        }
    }
}

This command would place the chi package under /vendor/chi instead of /vendor/github.com/pressly/chi. Therefore, packages could be imported as:
package main

import (
  "net/http"
  "chi"
)

Since golang/dep treating third-party packages as first-class citizens, I believe that import syntax should reflect this cultural change. Of course, there are many issues that I did not consider and could make this difficult. All that being said, thank you for your time and for listening to this idea!

Thanks for making the suggestion :) I quite understand the impulse for it, and have had a similar thought myself. Unfortunately, though, I don't think this would be workable. There's a few things I could point to, but one is most clearly disqualifying: conflicts. There are (at least) four classes of conflict such aliases would allow.
Let's think about how this would have to work. A manifest declares an alias, which is basically a mapping from a project root (chi) that's seen in the code, to the original, proper project root (github.com/pressly/chi). Because you're going to commit code with these aliased import paths, it's necessary that anything else depending on your project also know that github.com/pressly/chi is aliased to chi; otherwise, this becomes a backdoor to duplicating projects, which is something we try very hard not to do.
For ubiquity of the aliasing to be enforced and this backdoor avoided, it means the solver would have to lift aliases from dependent projects' manifests up, and apply it globally to anywhere github.com/pressly/chi appears in the depgraph. So, if any OTHER project in the depgraph ALSO relies on github.com/pressly/chi, then there's a potential for conflict if that other project either a) specifies no alias, or b) specifies a different alias.
That's two conflict classes. The third is, if you alias to chi, and something else in your depgraph ALSO aliases to chi, but it's a totally different project (github.com/foo/bar). And the fourth - what if you alias to bytes (stdlib), or github.com/something/real? Is that OK, or is it an error to supercede something else in that way?
Import paths provide canonicality for names, and it's important we maintain that as much as possible.
Also, let's not forget that we have to construct an on-disk representation of your deps in vendor/ that will actually compile. If you were to alias github.com/pressly/chi as just chi, then that means it would have to live at vendor/chi for the imports to be picked up. Now, that project seems to have rather isolated packages, but if it has internal imports (e.g. github.com/pressly/chi imports github.com/pressly/chi/middleware), then those import paths would have to be internally rewritten in the code in vendor in order to make the package internally consistent. And we don't do import path rewriting, ever. If nothing else, it changes the text in files, which makes hashing the contents of vendor even more difficult (#121).
So, yeah - nifty idea, but I can't see it working well. It's a superset of the problems that specifying an alternate source has, but with much less benefit - whereas alternate sources let you transparently swap in e.g. a fork, or more (#174). I can't think of a problem this solves apart from typing fewer chars.
I'm going to close this as a clear signal it won't work, but again, thanks for the idea, and please don't be discouraged from posting more ideas if you have them!

You've raised great points that in my haste I didn't consider (partially the conflicts in /vendor). Your explanation has taught me a lot about the complication model, and I'm really thankful for that ❤.

authors: davidbarsky sdboyer


text:
time.Unix(sec, nanonsec) has bizarre undocumented behavior for large values of sec

The biggest value that can be passed as the first parameter of time.Unix(seconds, nanos) is
math.MaxInt64 - 62135596800. Sample code: http://play.golang.org/p/Ck5Ny1W-ud
62135596800 is the number of seconds, accounting for leap days, that separate the start of the Go internal epoch (year 1) from the start of the Unix epoch (1970).
time.Unix() should not leak its implementation details in this way, or else the behavior should be clearly documented.
It does not seem unreasonable, given the current spec/docs, for someone to create a time.Unix(math.MaxInt64, 0) as a sentinel value, but this leads to strange and hard-to-debug behavior.

time.Unix(math.MaxInt64, 0) is expecting too much from
the undocumented implementation.
The internal epoch value is intentionally not publicly
documented.

authors: maxpoletto minux

repo: golang/go


text:
x/net/http2: Pushing?

I see that there is code to meet the spec to send push promise frames in the http2 library, however I was under the assumption that using these methods was not the intended purpose. Are there plans to add helper functions for pushing?

We use the issue tracker for bug reports and tracking.  Please ask questions on the golang-nuts mailing list.  Thanks.

authors: ianlancetaylor morganhein


text:
math/big: mistake in the definition of DivMod

The documentation for DivMod, in the big library, suggests that z.DivMod(x,y,w) sets z to the quotient q and w to the modulus m, where q and m satisfy

m = x - y*q  with 0 <= m < |q|

I believe that this should read

m = x - y*q  with 0 <= m < |y|

so that it agrees with the reference mentioned in the documentation for DivMod: Raymond T. Boute, “The Euclidean definition of the functions div and mod”. ACM Transactions on Programming Languages and Systems (TOPLAS), 14(2):127-144, New York, NY, USA, 4/1992. ACM press.

I can't tell: is this a problem with the docs, or a problem with the code?

Code looks OK to me.

Yes, if the operation is q=x/y then clearly the remainder (sorry, modulus) must be < y, not < q. Thanks.

authors: TomCoates8161 ianlancetaylor rsc


text:
crypto/x509: Certificate Name Constraints starting with "." not validated properly

What version of Go are you using (go version)?
go1.6.2

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

What did you do?
A certificate with domain test.example.com is validated with an intermediate certificate having a name constraint of .example.com.

What did you expect to see?
The certificate chain should be valid. A name constraint .example.com should allow all subdomains of example.com, but not the domain itself (according to the RFC).

What did you see instead?
The certificate chain is not considered valid (x509: a root or intermediate certificate is not authorized to sign in this domain), while test..example.com works.
The problem is the check at https://github.com/golang/go/blob/master/src/crypto/x509/verify.go#L172, that enforces a . as the last character of the domain prefix (that's why test..example.com is considered valid).

/cc @agl

This looks like an off-by-one in crypto/x509/verify.go:
			if opts.DNSName == domain ||
				(strings.HasSuffix(opts.DNSName, domain) &&
					len(opts.DNSName) >= 1+len(domain) &&
					opts.DNSName[len(opts.DNSName)-len(domain)-1] == '.') {
				ok = true

I've prepared a fix: https://github.com/JonathonReinhart/go/commit/ca0494a616d358193e8817c01ae732a33e08d9a7

@JonathonReinhart: Your fix would disallow subdomains for rules without a leading dot. The proper check should be the existing + the leading dot case:
            if opts.DNSName == domain ||
                (strings.HasSuffix(opts.DNSName, domain) &&
                    len(opts.DNSName) >= 1+len(domain) &&
                    opts.DNSName[len(opts.DNSName)-len(domain)-1] == '.') ||
                (domain[0] == '.' && strings.HasSuffix(opts.DNSName, domain) {
                ok = true

Interesting - there appears to be a disconnect between the RFC and many examples of x509 Name Constraints (for DNS names).
I was mistakenly looking at (and quoting) the following paragraph from section 4.2.1.10 of RFC 5280 which says:

For URIs, the constraint applies to the host part of the name.  The
constraint MUST be specified as a fully qualified domain name and MAY
specify a host or a domain.  Examples would be "host.example.com" and
".example.com".  When the constraint begins with a period, it MAY be
expanded with one or more labels.  That is, the constraint
".example.com" is satisfied by both host.example.com and
my.host.example.com.  However, the constraint ".example.com" is not
satisfied by "example.com".  When the constraint does not begin with
a period, it specifies a host.

However, this applies to URIs.  My CA certificate (and essentially all others using Name Constraints) looks like this:
            X509v3 Name Constraints: critical
                Permitted:
                  DNS:..example.com

Because this is a DNS Name (and not a URI), that paragraph does not apply. Instead, the following paragraph does:

DNS name restrictions are expressed as host.example.com.  Any DNS
name that can be constructed by simply adding zero or more labels to
the left-hand side of the name satisfies the name constraint.  For
example, www.host.example.com would satisfy the constraint but
host1.example.com would not.

Interestingly, this never says anything about a leading period. I interpreted a leading period to mean valid certificates MUST have at least one additional leading label. (e.g. the name example.com does not satisfy the constraint .example.com). The RFC appears to leave the behavior associated with a leading period on a DNS name constraint undefined.
Here are examples that include a leading period:

Constraints: what they are and how they’re used (Windows PKI Blog)
Using Name Constraints (Technet)
CAs, Name Constraints, and a Business Opportunity (karl.kornel.us blog)
Hellenic Academic and Research Institutions Cert. Authority 2011 (An actual CA cert, trusted by Mozilla, Microsoft, and Apple)

Other examples that don't specify:

Name Constraints in x509 Certificates
x509v3_config (OpenSSL docs)

Finally (after hours of digging), an example that never uses a dot, and explicitly implies that DNS:bad.com means *.bad.com:

test/certs/setup.sh (OpenSSL test suite)

Relevant OpenSSL source (which I'm still digging through):

check_name_constraints()
crypto/x509v3/v3_ncons.c

Interesting: The code I've shown above is from the current OpenSSL master branch. nc_dns() was changed via https://github.com/openssl/openssl/commit/77ff1f3b8bfaa348956c5096a2b829f2e767b4f1, after bug report RT3562 (the 3662 cited in the commit is a typo). That change allowed leading dots in DNS name constraints, even though the RFC doesn't specifically allow it.
So @floridoo I'm essentially in agreement with you now. My previous understanding, that the dot must be present for a subdomain match, was incorrect.
crypto/x509 should do the same thing OpenSSL did, and allow the dots to be present, but not required.  I think https://github.com/golang/go/issues/16347#issuecomment-235190507 is quite messy, and this conditional is too complex to be one expression. The way OpenSSL does it looks much cleaner, IMO.

Relevant code in NSS: https://github.com/racktopsystems/nss/blob/master/lib/certdb/genname.c#L1203

Please let me know what you think of https://go-review.googlesource.com/30155.

30155 looks good to me.

authors: JonathonReinhart agl floridoo quentinmit


text:
x/tools/cmd/goimports: permission denied causes imports to stop

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jeffwilliams/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.7/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.7/libexec/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jc/ctl0fvhs34v21m_hg1z3w0zh0000gp/T/go-build379181819=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

One unreadable dir in GOPATH will cause the walk to fail and ignore all directories after that point, with a confusing message like:
2016/08/26 15:39:24 goimports: scanning directory /Users/jeffwilliams/go/src: permission denied

In this case, it was a test dir with permissions 0000:
/Users/jeffwilliams/go/src/github.com/wherethebitsroam/cloudcp/test/fred/dave

Admittedly, moving this test content into testdata where it should have been fixed the problem. But this is not very obvious given the error.
The relevant code was golang.org/x/tools/imports/fastwalk_unix.go:
fd, err := syscall.Open(dirName, 0, 0)

What did you expect to see?

goimports to skip the directory with permission denied. Or at least give an error with the directory which was the problem.

What did you see instead?

goimports failed to insert the imports for known packages in the file being processed.

Hey guys, happy to help but what should the behaviour be:

Skip the directory silently
Skip the directory and give a error message
Stop processing and have error message give the directory with the error

I would lean towards 2.
Jeff

1 or 2 seems better. goimports is frequently run from editor hooks, so one important question is how they handle output on stderr. Will be it be presented to the user? If not what's the point, but if so, will it be annoying? Will output on stderr be interpreted as failure and prevent formatting? Given all that I personally lean towards 1.

goimports has a verbose (-v) flag as of recently. Let's be skip & silent by default, but in verbose mode let's show a warning (but still skip it).  Because in verbose mode, presumably a human is running it, instead of an editor, and they're more likely debugging and seeing stderr.

Hi guys!
I'm a noob but totally available to fix this issue. Could you please let me know whether I need to mark myself as the assignee (and how), so that we can avoid duplicate work? Or do you follow a different path?
For the fix: ideally we could use all three strategies proposed by Jeff. Using option 3 (strict mode) during the import process would let the user decide if the issue is a severe one or can be ignored.
For option 1 (skip & silent) I'd rather give a warning message instead. That would be useful while investigating a build-automation log. Hiding such an error could be very painful to debug.

@luigi-riefolo, no need to use Assignee. Just send a change and drop a comment here with the URL to the Gerrit review.
Please use the design I proposed in https://github.com/golang/go/issues/16890#issuecomment-242926004 ... we can't be interactive or chatty by default, as we're likely running underneath an editor process.

@jeffw-wherethebitsroam I've tried to reproduce the issue using the same environment but with no results.
I've chmod the dir wherethebitsroam/cloudcp/test/fred/dave to 000 and the error message for both
go build test.go and goimports -v -e test.go is:
open /Users/luigi/Workspace/Go/src/github.com/wherethebitsroam/cloudcp/test/fred/dave: permission denied
As you can see it specifies the 000 dir.
So could you please be a bit more specific on the issue?
Thank you.

Hi Luigi,
Are you running the goimports from the cloudcp directory? If so, can you try from another directory?
Otherwise, I have attached a little example.
gotest.tar.gz

extract the tar.gz
set your GOPATH to the new directory
chmod 0000 gotest/src/github.com/wherethebitsroam/cloudcp/test
cd gotest/src/github.com/wherethebitsroam/test
goimports -v -e main.go

Which should show an error like:
2016/09/05 09:53:00.769508 goimports: scanning directory /Users/jeffwilliams/gotest/src: permission denied

Jeff

Thank you @jeffw-wherethebitsroam for the accurate example.
I've run each step and did get the import for "fmt":
GOPATH=$HOME/Downloads/TEST/gotest goimports main.go
2016/09/05 15:56:26 goimports: scanning directory /Users/luigi/Downloads/TEST/gotest/src: permission denied
package main

import "fmt"

func main() {
	f := cloudcp.DoStuff()
	fmt.Printf(f)
}

So apparently we do get the imports for anything after the 000 dir. But please correct if I'm wrong.

Probably not a great example, it was just to show you the misleading error line.
The fmt package is in GOROOT. This bug will only affect packages in GOPATH which would have been walked after the permissions error occurs.

@jeffw-wherethebitsroam thank you for being specific. I'll take in count your observation for my next tests.

@luigi-riefolo are you still planning on fixing it ? if not I may try to pick it up
Cheers

@asticode you can pick it up

@bradfitz I have tried to fix it according your ideas here and in that last CL, but I fail to see good solution without refactoring many things and adding ugly hook into fastwalk.
What if we just improve error report printed? It will help users to see the problematic dir:
goimports: scanning directory /Users/t/go/src: permission denied while reading dir: /Users/t/go/src/imp/cloudcp/test

Let me know if I need to create separate issue for that. Thank you!

authors: asticode bradfitz jeffw-wherethebitsroam josharian luigi-riefolo ysmolsky


text:
Proposal: allow conversion between recursively equivalent types

Summary: I propose we consider relaxing struct conversion rules so that when converting composite data types (eg structs and slices), we recursively attempt to convert element types as well as the fundamental types. This would simplify certain important use cases, such as the handling of (otherwise) identical protobuf messages.
1. Background: This proposal is inspired by the (accepted) proposal described in #16085, and heavily references that proposal, but has a significantly wider scope. For example, under the current type conversion rules, we can convert between the following two types:
type type1 struct {
	id int
}

type type2 struct {
	id int
}

But we cannot convert between the following two types, even though they are clearly identical, from a data perspective:
type id1 int
type type1 struct {
	id id1
}

type id2 int
type type2 struct {
	id id2
}

Unfortunately, this is very similar to the kinds of structures that are created by the Protocol Buffer compiler. This means that many structurally identical messages cannot easily be converted from one proto package to another.
Issue #16085 (Section 5) did specify that tags should be ignored recursively, for the purposes of type conversion, but this is only relevant for anonymous structs and therefore is not widely applicable. However, it does foreshadow the issues raised by the this proposal.
2. Proposal: The current spec defines that “A non-constant value x can be converted to type T [if, among other cases,] x's type and T have identical underlying types.” Instead, I suggest that this be changed to “x’s type and T have structurally equal (SE) types.” As one might expect, two types are SE if they meet any of the current criteria (including being of the same underlying type), or if they:

Have the same Kind (as defined in the “reflect” package), and
Have component types that are themselves SE.

To put this in concrete terms for each of Go's composite types:

Array, channel or slice: two array, channel or slice types are SE if their element types are SE. Of course, an array type is not SE with a channel or slice type or vice versa.
Functions and methods: two function types are SE if all the following conditions are met:

The corresponding parameters (by order of declaration) of the two functions are identically named and have SE types;
The corresponding return values (by order of declaration) of the two functions are either both unnamed or identically named, and have SE types.

Interfaces: two interface types are SE if they have the identical set of methods, and all corresponding methods of the same name are SE.
Map: two map types are SE if the keys and values are SE.
Pointer: two pointer types are SE if their base types are SE (this is already allowed by the spec, but as a special case).
Struct: two structs are SE if the corresponding fields (by order of declaration) of the two structs are identically named and have SE types.

In addition to changing the built-in language behaviour, we would also update the reflect package to match this change (Type.ConvertibleTo and Value.Convert).
3. Impact: as with #16085, this is a backwards-compatible change since it only loosens restrictions. The only exceptions to this are the behaviour of several methods from the “reflect” package, as described in #16085. Furthermore, the benefits should be greatly enhanced when dealing with protobufs, as described in the background.
4. Discussion: While the primary driver for this change was to simplify the handling of structures, I have included functions, methods and interfaces for completeness. However, if the inclusion of these functional elements causes problems, it may be acceptable to remove them from this proposal as well.
A further note should be made about method sets. Under the current rules, it is already possible to cast one type to another and completely change its method sets. As a result of the current proposal, the method sets of its element types may also change. For example, if x.a.Foo() was a valid call, and x is casted to y (a variable of a different but SE type to x), this does not necessarily mean that y.a.Foo() will be a valid call - or if it is a valid call, that it will refer to a method with the same signature, let alone semantics.
5. Alternatives: One alternative to allowing automatic conversion is for developers to continue to manually specify every field that needs to be converted. Another is to provide a library function, which would fail at runtime (instead of at compile time) if the two types were not SE. There would also be performance and memory considerations, as in #16085.
6. Implementation: TBD. In the compiler, we would need to generalize the type identity functions to recurse in the case of conversions. The spec would be updated with a new section on structural equality, and the reflect package would need to be updated to match the behaviour of the compiler.
Thanks to @griesemer for some preliminary comments on this proposal.

I'm not sure how I feel about this.  The recursive nature of this proposal means that we can explicitly convert between interface types that are entirely different, as in
type Seconds interface {
    Add() int64
}
type NanoSeconds interface {
    Add() time.Duration
}
var S Seconds
var S2 = NanoSeconds(S)

Of course the conversion does have to be explicitly, so presumably people should avoid doing that.  But the identical underlying type can be hidden beneath an arbitrary number of layers of function parameters.
I can appreciate the desire for MOVE CORRESPONDING but I'm concerned that this proposal goes too far.  And I don't know how to usefully limit it while still permitting what is desired.

I had similar concerns about interfaces - the proposal's really targeted towards data (structs, etc) and not functions. Would you feel better if interfaces were excluded?

Interfaces are just one aspect of the issue, because interfaces are just the dual of method sets.
// T1 represents a number of seconds.
type T1 int64
// Time returns a number of seconds.
func (t T1) Time() int64 { return t }
// T2 represent a number of nanoseconds
type T2 time.Duration
// Time returns a number of nanoseconds.
func (t T2) Time() time.Duration { return t }

type S1 struct {
    t T1
}
type S2 struct {
    t T2
}
var V1 S1
var V2 = S2(V1)

This conversion is accepted by your proposal even though the values have different meanings and the value methods have different meanings, they just happen to have the same underlying type.

Another case worth considering is oneof. For proto messages that use oneof, the proto-generated structs will contain an interface. For message Foo below, the message and field names are used in the interface's function name isFoo_Bar. So if you have two protos with the same oneof field structure but different e.g. top-level message names, they wouldn't be "SE" even if they have all of the same field names and effective types.
.proto
message Foo {
  oneof bar {
    string a = 1;
    int64 b = 2;
  }
}

.pb.go
type isFoo_Bar interface {
        isFoo_Bar()
}

type Foo_A struct {
  A string
}

type Foo_B struct {
  B int64
}

func (*Foo_A) isFoo_Bar() {}
func (*Foo_B) isFoo_Bar() {}

...

type Foo struct {
  Bar isFoo_Bar
}

@ianlancetaylor - yes, I agree with what you're saying. I feel that there might be a semantic difference between interfaces and method sets - even though they behave the same way - but if most of the community doesn't see it that way, then there's little point introducing a distinction. Plus, as @russ points out, generated protobuf code does use interfaces, so we would have to support them.
@russ, thanks for pointing this use case out, I had forgotten about it. Given the current protobuf generator, I think the options would either be to ignore the method names (and only verify compare the other aspects of their signatures), or else insist that the top-level message names had to be the same.
The first option seems too lax to me - if an interface contained more than one method with the same signature (ignoring the name), it would be either impossible or very brittle to determine which method in the first type should match to the second.
The second option is feasible but unfortunate, since nothing else in this proposal cares about the names of the types themselves. So this would reduce the usefulness of this proposal, and could cause working code to suddenly break if a developer added a oneof to their code.
If we can change the protobuf golang generator, we have a few additional options:

We could save the oneof field contents as an interface{} instead of the special-purpose oneof interface type, but then you loose all type safety for the possible types of the field.
The oneof field could be stored in the equivalent of a variant, although this would require runtime checks that only one value was ever populated at a time, as well as wasting memory. I don't know if this would break other kinds of existing code, though - it would depend if people were manually checking for the isFoo_Bar() method.

I don't find any of these options terribly appealing. My best guess would be to sacrifice oneof support - any messages that contained oneof fields would simply not be SE with any other message that might otherwise be SE but does not have the same name. Does anyone else have any suggestions?

Since we're mostly talking about generated code, why couldn't we
make the code generator help us to write the type conversion boilerplate?
I also feel this relax the type system too much and allow some potentially
dangerous conversions, like two types with different method set (and
different
invariants) but happens to be SE. I know, the user has to be explicit about
the conversion, but when writing the code, people might not remember that
the two structs have different invariants, and incorrectly assumes that if
the
compiler allows the code, it should be fine.

@minux - the code generator won't know the different types you want to convert to/from in the general case. However, I agree that (in theory) we could make some changes to the generator to simplify these use cases.

As Ian pointed out in https://github.com/golang/go/issues/18030#issuecomment-262583969, there is significant complexity lurking here. Part of the reason things are as locked down as they are is to make it OK for people to change their own types in specific ways (like adding struct fields) without fear of breaking other people's programs.
The type system does not seem like something we should modify incrementally one step at a time without very good reason. We had a good, specific, compelling reason for #16085. What is the specific, compelling reason here?

Hi @rsc , the compelling reason is being able to copy information between two identically written but distinct proto messages. However, allowing that kind of casting would break the use case you're mentioning - if one of the protos changed, the code would break.
Given the complexities, perhaps this isn't worthwhile - maybe an extension to protoc that could provide translators between two specified protos would be better if there's sufficient demand.

OK, let's decline this until there is a more compelling reason.

authors: adrianludwin ianlancetaylor minux rsc russ-


text:
proposal: spec: add decimal float types (IEEE 754-2008)

I know there was a proposal about decimal as an std package.
But according to https://en.wikipedia.org/wiki/Decimal_floating_point, there have already been multiple compliant hardware implementations, and supported by several commonly used programming languages. I think having decimal as first-class citizen like the other part of the IEEE-754 standard makes sense.
The upside is we don't have to spend the effort on API design upfront to make it useful like math.big. We can just limit the scope of the operations only within the Go's existing arithmetic operators. Designing a good numerical/arithmetic API beyond the conventional operators is harder and can be deferred later with no harm.

Related: #12127, #19770

I think this proposal can be implement outside golang runtime and builtin.
Does any implement exist?

@bronze1man Yes, there are thirty-party packages and proposals for including one into std. But this proposal is about adding fixed-size decimal floating point types, along with the existing binary floating point types using the existing arithmetic operators, rather than introducing a new package.

Adding onto this - If we had operator overloading, this could be implemented as a 3rd party lib instead

Some useful information in:
http://open-std.org/JTC1/SC22/WG21/docs/papers/2014/n3871.html

It would help to have a clear idea of who would want to use these new types.
It would also help if the proposal spelled out the new types and how untyped constants would be handled.

It would help to have a clear idea of who would want to use these new types.

From manually polling people who use my (big) decimal package:

Banks
CAD
Numerical analysis
Databases (see: cockroachdb/apd)
E-commerce sites
Bitcoin sites

Thanks!  But we shouldn't confuse big decimals with this proposal, which is for fixed size decimals.  It's not obvious to me that people who use the former will want to use the latter.  For example, the fact that people use the math/big Rat type does not suggest that we should add a rational number type to the language.

I think there's more overlap than it would seem. A 128-bit decimal gives you 34 significant digits which is plenty for most regular usage, like wanting to perform accurate monetary arithmetic.
And, as Brad's link mentioned, four of the six listed languages have only big decimals. So, I think it's somewhat germane :)
A point to consider: if decimals are added there will likely need to be support in the math/ package, like math and math/cmplx.

In this proposal, i expect only fixed size decimals, and i need them for financial computations.

For people like me who know nothing, can you expand on why decimal floats are better for financial computations that 1) ordinary floats; 2) the int64 type?  Just pointing to a paper would be great.  Thanks.

Section 1.1 discusses a few reasons here - http://speleotrove.com/decimal/IEEE-cowlishaw-arith16.pdf

That PDF is good. His FAQ sections is good as well: http://speleotrove.com/decimal/decifaq1.html#inexact
TL;DR: floats are inaccurate because they can't exactly store base 10 numbers.
Integers (representing the smallest currency amount, e.g. 1 cent USD) work well, but can be annoying to use for anything other than basic arithmetic.

In my company, we used all 3 alternatives:
decimal in c/c++
int64 in go, c/c++,
float in c/c++, perl
floats are not exact numbers, making some elementary calculations can give surprising ( for financial people) results.
just storing and loading float64 from database (oracle use some sort of decimals for numbers) can lead to inexact value, and some financial applications are 90% database io.
int64s mostly works, but are cumbersome (IMO), in all IO to external systems (including databases) you must convert them to "right" form.
hope this is  useful

@ianlancetaylor
Regarding why floating decimal is different than floating binary, please have a look at https://www.exploringbinary.com/why-0-point-1-does-not-exist-in-floating-point/
I am not an expert on this subject either. What motivated me to the proposal, besides some ergonomic issues, is when I was working on a financial application using Go, I had to deal with quantities, prices, and other stuff related to money.
There are decimal libraries in Go that support basic arithmetics. But when I want to combine them with say, https://github.com/go-gota/gota and the like, it seems to be tough, if not impossible, to integrate with the functionalities provided by those data processing libraries with an arbitrary composite type. In other words, if I want to implement a matrix or statistics library, I would need to make them support passing third-party numeric types. I am not sure how that should be done if it were indeed feasible.

As I can personally testify, the 128 bit decimal type is great for writing financial applications in C# which need to be accurate to the cent, penny etc. as you don't have to worry about the rounding errors which can accumulate if you use their float or double types. The results are always exact even for the largest amounts likely to be encountered in practice.
The alternative is to always work in cents using their 64 bit long type and then div/rem by 100 when you need to display or store the final result. However, this is both tedious and error prone and may even not be large enough when you're dealing with some currencies.
The twin drawbacks are the extra memory required and performance - decimal is much slower than double which in turn is slower than long. However, for most financial programmers this is a price worth paying for the convenience they gain from using decimal.
Back in pre-.NET days, VB6 etc. had a 64 byte Currency type which (IIRC) was scaled to 4 d.p. This was more than adequate for applications which didn't need to process large amounts of money and I always thought it was a mistake not to support it in .NET as well.
So, yes, if you'd like Go to appeal to financial programmers, I think that adding language support for fixed point decimal types would certainly be worthwhile and I'd suggest that 64 bit and 128 bit types (perhaps named dec64 and dec128 respectively) would be adequate as I can't see much point nowadays in having a 32 bit type as well.
As far as untyped constants are concerned, I'd suggest that they should work in a similar fashion to what they do now for the other basic numeric types. So literals would accommodate themselves to whatever type was required, there'd be conversions to/from the other numeric types and so on. However, no untyped numeric constant would have a default type of either dec64 or dec128. So for example, we'd have:
    a := 123.45              // a is float64
    b := dec128(123.45)      // b is dec128
    var c = dec64(a)         // c is dec64
    d := b + 6               // d is dec128
    var e dec64 = 678.9      // e is dec64

I believe most people can agree that there is a need for decimals. The question is whether the standard library should provide it or whether it should be a primitive type.
The shortcoming of making decimal as a third-party package is that libraries working with it are hard to cooperate. For example, a package provides finance calculations based on a particular decimal type would not be naturally composable with another decimal type belonging to a time series data store. Then, it would create a situation where a financial application might have to include multiple decimal packages, and programmers have to write conversion code between them, which is clumsy and inefficient.

As @deanveloper  indicated earlier, the lack of operator overloading in Go would also be a problem if decimal support were provided as a library rather than built into the language.
In fact, I suspect most folks would prefer to stick with what they currently do rather than having to deal with named methods for the basic arithmetic operations. Whilst it's true that you have to do this already for 'big' number arithmetic, the difference is that you have no choice if you need to deal with such numbers.

I have just stumbled upon this striking example:
https://play.golang.org/p/w-e7BDgo734
Trying to round 35.855 to 2 decimal places gives unexpected result.
Can be embarrassing in a business app.

For people like me who know nothing, can you expand on why decimal floats are better for financial computations that 1) ordinary floats; 2) the int64 type? Just pointing to a paper would be great. Thanks.

@tc-hib Thanks.  I understand that floating point numbers behave in ways that people find surprising (https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html).  That's not the question I was trying to ask.

Oh, sorry I didn't get it.
Decimal type is a true silver bullet for all of us who don't need to save every cpu cycle, yet have to follow strict business rules. We simply use it everywhere. Once we've done that, it is really hard to go back, so we would always prefer translating our formulas to method calls rather than using ints or floats.
There are many legacy business apps (in COBOL for example, quite old), Excel sheets, etc. using decimal arithmetic as fluently as integers. I think Go would be a good fit for modernizing all those commerce specific apps thanks to its simplicity, but it would require "translating" every calculation carefully.
I believe using integers and floats is trickier than it seems, because we often have varying number of decimal places (3 for purchases, 2 for sales, and some intermediate calculations not rounded, such as taxes and commercial agreements). If we're only adding and multiplying numbers, or dividing by multiples of 5 and 2, users expect exact results. In fact, they expect same results as Excel sheet or pen and paper.
I must admit i don't have much experience in workarounds, because I've always been working with decimal types, and we nearly never use binary floats, as strange as it may seem.

The upcoming "Structured Field Values for HTTP" RFC will have a decimal type. To support structured headers and trailers properly in the stdlib, support decimal floating point will be mandatory.

https://www.reddit.com/r/golang/comments/l4x9zs/introducing_gofinancial/
Hopefully, the upcoming compiler refactoring for type parameters would make this more likely to happen as well.

https://www.reddit.com/r/golang/comments/l4x9zs/introducing_gofinancial/
Hopefully, the upcoming compiler refactoring for type parameters would make this more likely to happen as well.

I am new to Golang and found it surprising that after more than a decade maintainers aren’t convinced yet that having native support for decimals is essential for many applications.
Is there a serious drawback for adding decimals?

Well, it's a lot of work.
And having reread the comments, I see that there is still no clear statement of exactly what types should be added to the language.  I have a feeling that different people on this issue are talking about somewhat different things.

This proposal is about floats as defined by IEEE 754-2008, and I think most of us would be happy with only a 128 bit float.
It's good enough for most uses, just like binary floats are good enough for most uses except for correctness in decimal (human) representation.
I wouldn't expect a built-in type that "works like an int" to support arbitrary precision.

For a language proposal we need to be precise.  You are suggesting adding decimal128, but not decimal32, decimal64, or float128?

I believe decimal128 would suffice, yes, and it would be a huge improvement for all business apps, avoiding a lot of subtle bugs, especially for beginners.
Having a one size fits all type has proven to be worth it in several languages.
C# has decimal, which is a decimal128. It is widely used in business apps.
Having decimal64 or decimal32 seems like premature optimisation to me. Once we use decimal floats, we've usually already accepted their cost. Most of the time we'd rather have fixed point decimals for best performance anyway.
Having a float128 doesn't solve the same issue, that is, correctness in some representation. And I don't think the need would be as common as for the decimal type.

Integers (representing the smallest currency amount, e.g. 1 cent USD) work well, but can be annoying to use for anything other than basic arithmetic.

@ericlagergren Ah, you mean e.g. accrued interest, math.Pow, etc.?

To whom it may concern: there is a new, decent decimal128 implementation: github.com/woodsbury/decimal128. I've tested it and it's probably the best. I don't understand why the author hasn't advertised this package anywhere yet.

To whom it may concern: there is a new, decent decimal128 implementation: github.com/woodsbury/decimal128. I've tested it and it's probably the best. I don't understand why the author hasn't advertised this package anywhere yet.

That any better than the shopspring one that's been around for years?

That any better than the shopspring one that's been around for years?

It is better because  it is fixed size (128 bits), not arbitrary precision. If i  want to do scientific calculations, i will use  arbitrary-precision, but for financial i want fixed type.

To whom it may concern: there is a new, decent decimal128 implementation: github.com/woodsbury/decimal128. I've tested it and it's probably the best. I don't understand why the author hasn't advertised this package anywhere yet.

That any better than the shopspring one that's been around for years?

Yes. Just make benchmarks for your case, and you will see the difference.

To whom it may concern: there is a new, decent decimal128 implementation: github.com/woodsbury/decimal128. I've tested it and it's probably the best. I don't understand why the author hasn't advertised this package anywhere yet.

That any better than the shopspring one that's been around for years?

shopspring  uses *big.Int under the hood, which makes it quite slow if you need to perform some calculations (e.g. on an order book).

I agree that adding decimal128 is more practical than decimal32 or decimal64 or even float128. I would certainly love to have the full IEEE 754-2008 suite, especially since generics reduce some of the burden of supporting many numeric types in libraries, etc., but  having some decimal type is of considerable value. And if you were to pick only one, then decimal128 is clearly the forerunner:

This TigerBeetle article is a good example of where 128-bit decimals can shine: https://tigerbeetle.com/blog/2023-09-19-64-bit-bank-balances-ought-to-be-enough-for-anybody/ (there is also a reference to Go's lack of native support)

It was pointed out earlier in this thread, but note that .NET's original (recent) decimal implementation is not entirely equivalent to the decimal128 in IEEE 754-2008, but is nonetheless useful. There is a relevant discussion within this .NET runtime proposal: https://github.com/dotnet/runtime/issues/81376 (note also that the proposal has been approved!)

_Decimal32, _Decimal64, _Decimal128 have also been included in C23.

As mentioned elsewhere, support for decimal types is common in databases, is included in SQL and facilitated by the database/sql package (see also #30870).

Apache Arrow supports Decimal128 and Decimal256, though the Go implementation is currently using math/big which is unfortunate.

MongoDB and BSON support decimal128.

Initially my thinking was that #9455 should be a higher priority than this, but after reading through comments in both that thread and this, and after porting a fairly large code base off of big.Rat to the excellent github.com/woodsbury/decimal128 package, it has been so painful that I now feel strongly that some native decimal type is warranted. The math/big package is very useful but difficult to hold correctly and folks get tripped up by its strategy to avoid allocation. Packages like github.com/woodsbury/decimal128 provide a more natural interface, though allocations become harder to control. But for both approaches, they become an absolute chore to use and leak through libraries in uncomfortable ways -- at least for me.
complex64 and complex128 appear to be considered unusual but occasionally very useful, and have proven themselves warranted. I would expect decimal128 to prove to be far more popular and useful much more often.
I am left unsatisfied by the disarray of packages, sometimes compatible at the seams, but each never feeling whole. Working with currencies and combining big.Rat, x/text/... and github.com/woodsbury/decimal128, for example, certainly does not feel cohesive or straightforward. By comparison, complex128 is very pleasant to use. (There of course are a few issues to unpack here but the lack of builtin support is the most glaring by far)
Would a proposal to add a single decimal128 type be amenable? Is that effort more worthwhile independent of efforts to support the other IEEE 754-2008 types? I suspect such a separate proposal would be closed as a duplicate of this one. I certainly understand hesitation to introduce a new builtin type to the language as that has consequences that I struggle to comprehend completely, but I similarly struggle to understand what the bar is for this proposal.

Once you decide to implement decimal128, I imagine it would be relatively cheap to also implement decimal64 and decimal32.
Moreover, decimal64 would be much more efficient in time and space and have enough precision for the vast majority of use cases, so I suspect it would be more popular if it was available alongside decimal128.

authors: Al2Klimov Jalalx alanfo anzdaddy avdva bradfitz bronze1man deanveloper djadala dunglas ericlagergren ianlancetaylor lrewega myserg tc-hib treeder typeless


text:
Add os.ModeExecutable

What version of Go are you using (go version)?
go version go1.9 linux/amd64
What did you do?
Tried to determine if a file is executable
info, _ := os.Stat(file)
isExecutable := info.Mode()&os.ModeExecutable == 0

What did you expect to see?
os.ModeExecutable to equal 0111
What did you see instead?
os.ModeExecutable is not defined.

Executable for who?
We also don't have ModeReadable and ModeWritable, so why just executable?

@bradfitz makes sense

authors: bradfitz icholy


text:
cmd/link: missing arg stack map for functions defined in different package

What version of Go are you using (go version)?
tip (86a338960d)
Does this issue reproduce with the latest release?
yes
What did you do?
https://play.golang.org/p/4-OeiC68PO4
package main

import "bytes"

func growstack(n int) {
	if n > 0 {
		growstack(n - 1)
	}
}

func main() {
	var x, y []byte
	defer bytes.Compare(x, y) // on AMD64 bytes.Compare is implemented in internal/bytealg
	growstack(10000)          // trigger a stack grow
}

$ go run b.go
runtime: frame bytes.Compare untyped args 824634277936+56
fatal error: missing stackmap

runtime stack:
runtime.throw(0x1073aff, 0x10)
	/Users/cherryyz/src/go-tip/src/runtime/panic.go:598 +0x72
runtime.adjustframe(0x7ffeefbff518, 0x7ffeefbff630, 0x10b3be0)
	/Users/cherryyz/src/go-tip/src/runtime/stack.go:698 +0x578
runtime.tracebackdefers(0xc000000180, 0x1077300, 0x7ffeefbff630)
	/Users/cherryyz/src/go-tip/src/runtime/traceback.go:73 +0xda
runtime.adjustdefers(0xc000000180, 0x7ffeefbff630)
	/Users/cherryyz/src/go-tip/src/runtime/stack.go:733 +0x45
runtime.copystack(0xc000000180, 0x4000, 0x10c5f01)
	/Users/cherryyz/src/go-tip/src/runtime/stack.go:878 +0x17e
runtime.newstack()
	/Users/cherryyz/src/go-tip/src/runtime/stack.go:1063 +0x30e
runtime.morestack()
	/Users/cherryyz/src/go-tip/src/runtime/asm_amd64.s:481 +0x8f

The stack map of bytes.Compare is missing.
The compiler does generate the stack map
$ go build -gcflags -S bytes
...
"".Compare.args_stackmap SRODATA size=12
        0x0000 02 00 00 00 0e 00 00 00 09 00 09 00              ............
...

But it doesn't make into the binary. Apparently the linker dropped it on the floor?
This doesn't seem to be the case for assembly function defined in the same package.

Ok, for normal assembly functions, the obj package will insert a FUNCDATA which refers to the args_stackmap generated by the compiler from the Go declaration.
https://go.googlesource.com/go/+/master/src/cmd/internal/obj/plist.go#78
But this is only done for functions whose name starts with ""., which means, defined in the same package.
Maybe we can make the obj package also insert FUNCDATA for functions not defined in the same package as the declaration? Or the assembly writer is expected to write a GO_ARGS pseudo-instruction (https://go.googlesource.com/go/+/master/src/runtime/funcdata.h#34) for this kind of functions?

CC @aclements @randall77

I'd rather avoid the GO_ARGS solution.  We can certainly do that for internal/bytealg, but this problem would occur for anyone writing an assembly routine that is exported.  I don't want them writing a GO_ARGS declaration when we have all the information we need to construct it for them.
What happens if we just remove the "". restriction?  Does everything still work?  How much extra linker work is it?  Extra binary size?
Maybe we could restrict to functions which have a declaration but no body.  I'm not sure whether we have that bit in the data exported from a package, though.

What happens if we just remove the "". restriction? Does everything still work?

This doesn't work, since not all assembly functions have Go declarations. For them, it will result in undefined relocation targets at link time. I tried restricting to only functions whose name has a ".", but still some functions don't have Go declarations. Maybe we can establish a convention that if a function doesn't have Go declaration (therefore cannot be called from Go), it should not have "." (written as "·" in assembly source code) in its name? We could fix up the exceptions in std, but not sure what to do with code outside our repo.

Can we restrict to exported functions?  I think it would be reasonable to require that exported assembly functions have Go declarations.  In fact, I don't think it would work otherwise (unless assembly was calling assembly?).

I'd rather avoid the GO_ARGS solution. We can certainly do that for internal/bytealg, but this problem would occur for anyone writing an assembly routine that is exported.

I may be misunderstanding, but doesn't this also require that the assembly routine be implemented in a different package than its Go declaration? I'm okay with saying that end users don't get to do that.

@aclements Yes, I guess that's true.
In any case, I have a patch that restricts to exported functions, and it seems to work.  I'll mail it in a sec.

My patch didn't quite work, due to shared libraries.
When building runtime as a shared library, that shared library actually includes the implementations of bytes.IndexByte, for example.  That implementation now references bytes.IndexByte.args_stackmap as its args pointer map funcdata.  But that symbol doesn't exist when compiling runtime, and won't exist if you never use the bytes package.
Adding something like:
FUNCDATA $0, ·IndexByte·args_stackmap(SB)

to the bytes.IndexByte assembly fixes things.  Of course, that's probably the original GO_ARGS fix Cherry suggested.
I guess that's good enough to fix this bug, I'll whip up a CL.
There's the remaining question of whether my CL 122519 should be submitted.  I think it might still be a good fix for people deferring exported assembly functions which don't have package "". (Presumably people use pkg·Func as well as ·Func to declare their assembly functions.)

authors: aclements cherrymui ianlancetaylor randall77