text:
Mipmap creation is not threaded (for ImageTexture::create_from_image())

Godot version
3.3.3 stable.mono  (and 3.1.1 stable -> 3.4beta4)
System information
Windows 10, GLES3
Issue description
not sure on title
The mipmap creation in ImageTexture::create_from_image() (flags=1 or 7) runs on the main thread, even if the create_from_image() call itself is in a thread. This results in the application freezing for large resolution images (or presumably for weaker computers).
I am not certain if this is an issue, but I think it should be documented at least.
Steps to reproduce
a.jpg is a 12000x12000 image
	var p:String = "res://a.jpg"
	var t:Thread
	func test() -> void:
		t = Thread.new()
		t.start(self, "_test", 0)

	func _test(_u:int) -> void:
		var i:Image = Image.new()
		var it1:ImageTexture = ImageTexture.new()
		var it2:ImageTexture = ImageTexture.new()

		var e:int = i.load(p)
		if e == OK:
			it1.create_from_image(i, 1) # causes lag
			it2.create_from_image(i, 0) # does not cause lag (much less at least)
		t.call_deferred("wait_to_finish")

Minimal reproduction project
mipmap lag.zip

Does it actually run on the main thread? Or does it just block somehow?
Also, if the lag is caused by generating the mipmaps, I expected i.generate_mipmaps() to improve performance, but it does not seem so.

I have a feeling the lag has more to do with the fact that you call wait_to_funish immediately after generating the mipmaps. This tells the main thread to stall until your thread has finished running.

I have a feeling the lag has more to do with the fact that you call wait_to_funish immediately after generating the mipmaps. This tells the main thread to stall until your thread has finished running.

The actual implementation in my program (where I noticed the issue) has threads that only exit when the application closes, so wait_to_finish() is not the issue.

I think https://github.com/godotengine/godot/pull/51969 is wrong, because I see no code enforceing Image.generate_mipmaps to run on the main thread, and I see no reason it would be a problem to run on a different thread.

I think #51969 is wrong, because I see no code enforcing Image.generate_mipmaps to run on the main thread, and I see no reason it would be a problem to run on a different thread. I also see no good reason for it to be enforced in any way. As far as Image is concerned, there is no exotic threading issue there.

Indeed Image.generate_mipmaps() should run on the calling thread. Nothing about it enforces running on the main thread. As discussed in this issue, the issue OP is facing is that ImageTexture.create_from_image()  calls rendering server functions that have to run on the main thread in Godot 3.x. So it doesn't return until those functions are run.

As for ImageTexture, I havent fully followed nor use Godot 3 GLES3 renderer anymore, but I believe it isn't good at multithreading graphics resource creation. It is better with the Vulkan renderer though.

It is still relevant for the OpenGL3 renderer, for the Vulkan renderer everything should be fine running on the calling thread.
That being said, this issue is not about 4.0 documentation it is about a lag a user experiences in 3.x. Accordingly the 4.0 documentation issue should be tracked elsewhere

authors: Goblinth Zylann clayjohn mortarroad


text:
Godot Visual Script - 3.4 and 4.0 - Call Group Issue

Godot version
3.4 stable
System information
Ubuntu 20.04 LTS, Ryzen 2700, Gtx 1060
Issue description
The call group node gives errors and has no ports to input arguments. Occurs in both 3.4 and 4.0

There is also a problem with the fields disappearing when you reopen the project

.
Steps to reproduce
Put a node in a group, add a function to a script on that node, attempt to call it with the call group vs node from another script.
Minimal reproduction project
34_Call_Group_Issue.zip
You will have to drag out a fresh call group vs node and hook up the fields. (See above ScreenShot.)

https://github.com/godotengine/godot-proposals/issues/3036

source: gharchive/issue

authors: Anaxan Gallilus

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Export value shown twice.

Godot version
4.0.alpha13
System information
MacOS 12.5
Issue description
I am exporting a NoiseTexture using @export. It shows up twice in the inspector and removing the @export makes both disappear.
extends Node2D

@export var noise: NoiseTexture = null;

func _ready():
	pass # Replace with function body.

func _process(delta):
	pass

Steps to reproduce

Create a new node with a script.
Add a @exported NoiseTexture
Two values appear in the inspector.
Change the type to a String
Only one input changes type.

Minimal reproduction project
My Project:
Archive.zip

Duplicate of #63668.

authors: kleonc mageowl


text:
Leaving dock area releases mouse button

Godot version
0c5f254
System information
Windows 10 x64
Issue description
Seems like some recent regression.

Tested only with tile editor.
Steps to reproduce

Open tile editor (add tilemap with tileset etc)
Drag to select tiles
Leave tile area while dragging
The drag is cancelled

Minimal reproduction project
No response

Might be related to a similar behavior I found with sliders in the inspector: if I grab one and dare moving the mouse outside of the slider's rectangle, I loose grab of it.

@Zylann That is tracked in #63544 and allegedly fixed in #63598.

To be honest, the root cause might be the same as the one for the slider (the dragged widget will be set invisible, because the cursor went out of the view-port), but the slider fix will probably not fix this.

authors: DNKpp KoBeWi Zylann tinmanjuggernaut


text:
Godot Mono External editor setting is broken

Godot version
4.0
System information
Windows 11
Issue description
(4.0 rc1) I opened godot 4 rc1 and when selecting external editor I get a text box with the number 4. Changing the numbers has no impact and putting text reverts to numbers. (I opened a beta 17 project, not sure if it is important)
Steps to reproduce
-Open godot mono rc1
-Select external editor
Minimal reproduction project
None

The editor settings were renamed in https://github.com/godotengine/godot/pull/72061.
The setting is still showing up because it is set in editor_settings-4.tres (see https://docs.godotengine.org/en/latest/tutorials/io/data_paths.html#editor-data-paths for the path to that file).
There's a proposal about removing obsolete/undefined settings: https://github.com/godotengine/godot-proposals/issues/3526.

Duplicate of https://github.com/godotengine/godot-proposals/issues/3526.

authors: Calinou MagnetBoi raulsntos


text:
Output doesn't scroll with new print messages and hides them sometimes

Godot version
v4.1.rc2.official
System information
Win10
Issue description
The output stops scrolling to new messages resulting in it being useless:

See video below as well.
This was confirmed by someone on discord too.
Steps to reproduce
Print a lot, in my case 1 - 2 times per _process() call.
https://github.com/godotengine/godot/assets/102416174/39391f0f-c300-4c3b-bb27-afa7df69c776
Minimal reproduction project
Any

Thanks for your report, this seems to be a duplicate of https://github.com/godotengine/godot/issues/78899 (which covers two problems, one of which is yours).

authors: Swarkin YuriSizov


text:
[3.0] viewport screen capture doesn't work

Operating system or device - Godot version: Linux, Godot 3.0 (master)
Issue description:
Getting a screen capture from any viewport results in empty data.
Steps to reproduce:
get_viewport().queue_screen_capture()
yield(get_tree(),"idle_frame")
yield(get_tree(),"idle_frame")
var i = get_viewport().get_screen_capture()
i.save_png("user://cap.png")

Now queue_screen_capture() and get_screen_capture() don't even exist.

Haven't tryed from gdscript, but you should be able to call 'viewport.get_texture().get_data()' to get a reference to Image wich you can save. Also calling 'image.lock()' and image.unlock()' should let you get (and maybe set?) pixels.
This is how it works for color picker when picking a color from the screen:
https://github.com/godotengine/godot/blob/master/scene/gui/color_picker.cpp#L428

@MarianoGnu you are correct, the issue is only that ViewportTexture::get_data() is not bound to GDScript, but that should be easy to solve.

authors: MarianoGnu germanalen nunodonato vnen


text:
--dump-extension-api-with-docs produces incorrect documentation for some data types

Tested versions
4.3-dev, d76c1d0e516fedc535a2e394ab780cac79203477 the first commit after 4.2
System information
MacOS Sonoma 14.0
Issue description
MacOS
Steps to reproduce
Run the Godot editor with the option --dump-extension-api-with-docs
And it seems to be filling some stubs for Vector operations with the stock documentation for it
Consider Vector4i, the operator '/' instead of producing this:
Divides each component of the [Vector4i] by the given [float].

Gets this node:
Divides each component of the [Vector4i] by the given [int].

Judging from my tool, this only happens for the Vector types:
 generated-builtin/Vector2.swift                     |   18 -
 generated-builtin/Vector2i.swift                    |   15 -
 generated-builtin/Vector3.swift                     |   30 --
 generated-builtin/Vector3i.swift                    |   15 -
 generated-builtin/Vector4.swift                     |   18 -
 generated-builtin/Vector4i.swift                    |   19 -

Minimal reproduction project (MRP)
No need for a project.

CC @rburing

Consider Vector4i, the operator '/' instead of producing this:

the operator? There are two divide operators? Are you sure you are generating your output correctly?

It does indeed use the same description for each operator, will look at the generation code and see if there's some issue there

Sorry, I overlooked this. I can fix it, but if you're already on it then go ahead.

You go ahead, I was just looking into it 🙂

Gave up on my tweaking with this, but one observation for fixing this:
The extension generates operators that isn't in the documentation, for example String operator% generates for each type, but the documentation only contains one, for Variant (so the check needs to do some check for this)

authors: AThousandShips akien-mga migueldeicaza rburing


text:
Fix the order of transformations for selection bounding box in spatial editor.

This was broken in #7438 where the ordering of scaling in Basis::scale() was fixed, but this line of code (which relied on the incorrect behavior) wasn't updated correctly.
Fixes #9894.

I hope I haven't missed any other similar piece of code which relied on the old incorrect behavior of Basis:scale and Basis:rotate... (which is not unlikely, I'm afraid)

source: gharchive/pull-request


text:
Fix EditorSettings saving on draw calls

Closes #22097.
This, combined with #22234 should ensure that everything in the CreateDialog is working fine regardless of whether it is receiving the signal.
I am a little confused though as to why EditorFileSystem::update_script_classes is being called every draw call though. Someone who's more familiar with how the file caching system works will need to determine whether that's really necessary (since presumably it should only need to be called any time a script file has actually been modified).

I do question how this fixes EditorSettings being saved every draw call, like the title claims. More or less it's just removing a signal from update_script_classes that's no longer needed. I haven't looked at the code in-depth like you have, but isn't update_script_classes (which is still being called every draw) what's causing the settings to be saved?
Personally, I don't see a reason why updating script classes needs to happen every draw, and I get the feeling it was more a hack for the sake of caching, if it's for caching reasons. Maybe we could remove the call to update_script_classes from the draw code?

@LikeLakers2 I'm not opposed to doing that as well (cause it doesn't make sense to me for it to be happening every draw call), but in the particular Issue that this closes, the call stack supplied in the OP shows that the save is occurring from the _update_create_root_dialog() call, triggered by the "favorites_updated" signal emission that is only happening every draw call in the first place because of the "update_script_classes" signal emission.
I'm not even sure if it's happening every draw call anyway. In editor_file_system.cpp, the _queue_update_script_classes() method gets called in several places, pretty much any time it's NOT actively scanning changed files (so basically, if we are not scanning and updating script files, then immediately re-scan and update script files, or something like that). This may have been confused with draw calls, but I also don't know exactly how to make it only trigger when it's really necessary.

That's... very confusing. I don't think there's any reason to always be re-scanning, not even for the sake of caching (the whole point of caching is to avoid re-scanning every time you need a piece of information).
That being said, I would personally leave fixing that for another PR, to avoid causing any sort of regression with this PR. Unless, of course, someone else says you should try to fix that in this PR too.

@LikeLakers2 ikr. I'll just leave that to someone who better understands how the heck that system works. XD

Thanks!

authors: LikeLakers2 akien-mga willnationsdev


text:
Enable ProGuard for Android release builds to decrease APK size

This reduces the size of release APKs by about 300 KB on average.
I'm marking this as a draft since this will likely conflict with third-party modules in its current state. How could this be integrated so modules can supply their own ProGuard exclusions? (cc @megasoft78)
Also, this should be tested when Mono support is enabled to check for bugs that could appear when using ProGuard alongside. (cc @neikeq)
Other than that, I can confirm it works when exporting the Platformer 2D demo.
If this is merged, we should also update the documentation to mention that release export templates use ProGuard. Since stack traces will be obfuscated, this will require developers to deobfuscate them using the appropriate mapping tools. See ProGuard and obfuscated stack traces in this article.
Related to #18253.

Old `proguard-rules.pro` file from initial PR version (July 2019)
-dontwarn org.godotengine.godot.**

-keep class org.godotengine.godot.** {*;}
-keep class * extends java.util.ListResourceBundle {
   protected java.lang.Object[][] getContents();
}

-keepnames class * implements android.os.Parcelable {
   public static final ** CREATOR;
}
-keepattributes *Annotation*

-optimizations !code/allocation/variable

Does this only touch the Godot java code? I don't see how this would cause any problem only with mono enabled builds.

@Calinou one area to be careful, there are several java methods that are only used by the native layer, which ProGuard is usually not able to detect. As such, it may often be overly aggressive and remove these methods, causing hard to diagnose runtime crashes.

@Calinou Is this still desired? If so, it needs to be rebased on the latest master branch.

@aaronfranke Yes, but it needs more testing, especially with the new Android build system. We'll probably have to wait until people have started porting "real world" applications to Godot 4.

Rebased with @mbrlabs' proguard-rules.pro file included. Note that I haven't compiled APKs with and without ProGuard to compare binary size or check if the ProGuard'd APK still works.

How could this be integrated so modules can supply their own ProGuard exclusions?

@Calinou This page has the instructions for how library modules can define their own proguard files and include it as part of the build (search for A library module may include its own ProGuard configuration file).
So to complete this PR, it looks like there are a couple remaining tasks:

Update the Android plugin documentation to specify that plugin developers should define and include their module proguard files.
Create an annotation (e.g: @UsedByNative) to specify to proguard that certain methods/fields should be left untouched as they're being used by the native code.

If this is merged, we should also update the documentation to mention that release export templates use ProGuard.

I don't think this should be applied to the generated templates. Given the way they're used for Android builds, it's likely to create issues.
We should make this an option for the custom Gradle based build (off by default) instead.

@Calinou and @m4gr3d is this still desired for 4.4?

I don't think it's needed at this point in time, but I'll defer to @Calinou as the author.

Closing per the above comments.

authors: Calinou aaronfranke fire m4gr3d neikeq


text:
Improve GradientTexture to allow more advanced types of fills

Pull Request sponsored by IMVU
This PR allows to create different linear and radial 2D textures with repeat and mirror options, while keeping the previous property values as default (2048px width, 1px height and left to right linear fill). Some examples bellow:

Bugsquad edit: This closes #18387.

clang format is wrong, i forgot to install the hooks

force-pushed to fix formatting

You should also bind the enums in _bind_methods.
If @reduz agrees with the feature, I'd ask you to add the matching documentation too (run godot --doctool . to generate the template).

bound the constants and ran doctool, but i didn't get any diff related to documentation, and the new properties was already visible in app

bound the constants and ran doctool, but i didn't get any diff related to documentation, and the new properties was already visible in app

You probably ran doctool wrong, since you bind new methods, there should be new documentation to fill. You should:

Compile the PR
Then run bin/your.godot.bin --doctool . (the dot at the end is relevant)

At the very least, making GradientTextures taller and rotatable would be helpful for prototyping (as you can use them instead of texture files or good old icon.png). See #29748.

With the height option it would be nice as well for making Light2D masks without having to leave the editor.

Gradient textures are very useful for TextureProgress, but I was always bothered by them not having height (and it was pretty much the only case when I used rect_scale).

Gradient Texture are also very useful as shader input. It eliminates the need to painstakingly create them in an external tool, which are not always equipped for the task.

I was seeking an easy way to create sky-like horizon gradients in 2D (which can be repeated with ParallaxLayer/Background), and this would fit nicely.

I want this.

@MarianoGnu  Is this still desired? If so, it needs to be rebased on the latest master branch. Since this PR is a feature proposal, you should consider opening a proposal which explains example use cases and how this approach will solve the problem better than other approaches.
Otherwise, abandoned pull requests will be closed in the future as announced here.

If this gets closed, I'm willing to pick it up.

I don't see much of an use case for this at all, and it makes using the class more complex for its intended usage, which is particle systems, visual shaders, color correction, etc.

@reduz, the current class is more like HorizontalGradientTexture, while the proposed class would be a GradientTexture indeed, so perhaps we just need two separate implementations at this point, or blame wrongly picked name for the class, because it just asks for those features, but currently the purpose of this class is certainly misleading for most users.
Do you know that GradientTexture used to be called ColorRamp in Godot <2.1 (while not being a texture)? Perhaps we just ended up with a bad rename... So perhaps lets rename it back to ColorRampTexture and implement an actual GradientTexture with this PR? Linking #16863. 😃
By the way, there's a similar class called CurveTexture, which is also one-dimensional and I suspect exists for similar described usage above (yet poorly documented as well).

Well, I just made finished another project, so I'll pick this up. I'll probably just make a completely new class and rename the old one to ColorRamp like @Xrayez suggested.

Well, I just made finished another project, so I'll pick this up. I'll probably just make a completely new class and rename the old one to ColorRamp like @Xrayez suggested.

Well it was renamed for a reason. Please make sure to have the approval of core developers before spending time on such changes which might be rejected.

In that case pinging @reduz who did the intial rename, and @akien-mga who merged karroffels PR for finishing the rename.
https://github.com/godotengine/godot/commit/c5dcbeb160b37d8ffd70061d76eb980756fe36b4 Suggests to me that ColorRamp ended up being renamed cuz the header was Gradient. Not because of deep semantic/functional reasoning.
Personally I'd be fine with a ColorGradient1D and Gradient2D. It also allows a Gradient3D to be added if someone would ever want that.
We shouldnt really touch CurveTexture with this discussion, its use case isnt graphics but maths. (it is necessary for it to be a "texture" only for use as a shader parameter)

@akien-mga yes, of course, sorry.

c5dcbeb Suggests to me that ColorRamp ended up being renamed cuz the header was Gradient. Not because of deep semantic/functional reasoning.

That's the other way around, ColorRamp was renamed to Gradient, but @reduz didn't bother renaming the file. I did it.

Personally I'd be fine with a ColorGradient1D and Gradient2D. It also allows a Gradient3D to be added if someone would ever want that.

Reminds me of animation nodes/resources (see AnimationNodeBlendSpace1D for instance), so yeah makes sense to go for that route.

Just ran into this issue. Thank you for the work!

Note that I've also been working on this in #42855 (adds a separate class for this), based on this comment: https://github.com/godotengine/godot/pull/31655#issuecomment-525472775.

close this because it implement in goost?

@CsloudX as has been recommended by reduz, pull requests for new features should not be created. However, both this and #42855 have a common proposal at GIP, see godotengine/godot-proposals#1677.
If you'd really like to see this feature directly in Godot, people must provide their own real-life use cases, and any limitations you stumble upon "must not be a speculation", as reduz says. By "real-life" I mean that you must have a project you're working on where you cannot proceed effectively without this feature. I hope this clarifies Godot's current development approach somewhat.

close this because it implement in goost ???!!!

This PR is like two years old, so I totally understand the decision to outright close the PR at its state by the author. If there's no decision making process going on, a person does not have to wait forever in the seek of feedback, it makes perfect sense to move on, don't you think? So, it's not necessarily because this feature is available somewhere else, it's because of lack of decision-making process regarding this feature.

authors: Byteron Calinou CsloudX KoBeWi MarianoGnu SekoiaTree TheDuriel Xrayez aaronfranke akien-mga avencherus tavurth


text:
Fix typo/spelling: "exisiting" -> "existing"

Specifically: "exisiting_script_removable" -> "existing_script_removable"
Note:

I haven't tested this change.

Not sure why this was missed by codespell?

Not sure why this was missed by codespell?

Not sure, it's actually included in codespell's dictionary since Jan 2018: https://github.com/codespell-project/codespell/tree/master/codespell_lib/data#L10069

Thanks!

authors: akien-mga follower


text:
Fixed IK rotation issue

Fixes #37095

Thanks!

Cherry-picked for 3.2.2.

authors: AndreaCatania akien-mga


text:
Exposed setters for sensors

set_accelerometer, set_gravity, set_magnetometer and set_gyroscope exposed to use it in scripts.
For example I want to port my module to GDNative but without this PR sensors data will not be available.

🐢

I'm not sure to understand what's the use case for having those methods exposed. Aren't those methods supposed to give information that comes from the OS itself? If I understand correctly, you'd want to have a proxy between a mobile device and your running instance on desktop with https://github.com/DmitriySalnikov/GodotRemote, so you need to pass the mobile OS sensor data to the Windows version? I guess that could make sense.
For the PR itself, there are a few changes needed:

PRs should be made against the master branch in priority. This change would also be valid in master for 4.0 and future releases, so if it's only done in 3.2 you won't have the feature anymore in future Godot versions, which would be a regression.
The commit log should be amended to be clearer and in plain English, see https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind
You should add documentation for the newly exposed methods (generate the template with godot --doctool .).

I'm not entirely sure I understand the use case as well as the need for this change in Input class.
This methods are also already exposed, aren't they?
master
https://github.com/godotengine/godot/blob/2abe996414b8b551e69e29461de3ff1bcaf5a28f/core/input/input.h#L296-L300
3.2
https://github.com/godotengine/godot/blob/3dd57a22df1fe431b71f2f4229cbff1107744658/main/input_default.h#L255-L259
Casting Input to InputDefault wouldn't cover it?

@akien-mga  I got it. But I do it later.
@naithar these methods are only available in c++, but it cannot be used in GDScript or GDNative.
I want to use it for debugging in the editor via the GDNative library. It's currently only available via a regular module.
(right bottom screen is a smartphone with my module)
https://youtu.be/LbFcQnS3z3E?t=76

authors: DmitriySalnikov akien-mga naithar


text:
Fix _shape_shape typos in TileData collision shape API

According to existing add_collision_shape() and remove_collision_shape(), the renamed methods should be set/get_collision_shape(), and not set/get_collision_shape_shape().

This PR is generously donated by Goost.

This is superseded by https://github.com/godotengine/godot/pull/49859 where I got rid of those functions. Shapes are getting replaced by a polygon instead, so those functions were removed.
But no, this was not a typo anyway, add_collision_shape and remove_collision_shape removes also the data associated with the shape (one_way and one_way_margin), while set/get_collision_shape_shape() only affect the Shape2D resource. I would have liked to find a better name but well, I did not.

The issue with method naming in #49859 still persists: _polygon_polygon(). If you really want to be explicit with this kind of API, most of the time, there are better alternatives, like: _polygon_points(), or _polygon_vertices(), or even with shape it would be _shape_resource().

The issue with method naming in #49859 still persists: _polygon_polygon(). If you really want to be explicit with this kind of API, most of the time, there are better alternatives, like: _polygon_points(), or _polygon_vertices(), or even with shape it would be _shape_resource().

Ah my bad you are right, I can change that then. Though I'm not sure there's a real problem with it TBH.

Though I'm not sure there's a real problem with it TBH.

To be honest, I submitted this PR immediately because I really thought there was an obvious copy-paste mistake regardless. Even Chaosus assigned the "bug" label to this PR. We should give good example for anyone reading the Godot's codebase, especially for new contributors, and especially when people tend to take existing Godot code as a reference for their own code.

Superseded by #49589.

authors: Xrayez akien-mga groud


text:
[3.x] Skip rendering of Light2D with zero size texture

Fix  #41756
Using light with not yet set texture cause spam of det==0 error to the console both in editor and in the game. (See issue #41756).
Error is coming from no yet set texture which seems to have size of (0,0) by default.
See explanation here: https://github.com/godotengine/godot/issues/41756#issuecomment-695756032
I tried to find best solution to this and this is the simplest one. I check if Light2D texture have 0 size and if it does then skip rendering this light.
Attached MRP provided by original author of the issue. Issue exist in 3.3.2 and this PR fixed it.
41756_issue_MRP.zip
I didn't noticed any regressions.
This issue doesn't exist in [master]

Please squash the commits together, we don't want to merge fixup commits if we can have the base commit "correct" from the start :)

Thanks!

authors: JestemStefan akien-mga


text:
Fix TranslatedLocal method in C# affecting the original transform

This bug was discovered by @bluenote10, thanks! https://github.com/godotengine/godot/pull/55923#discussion_r933804096

Thanks!

authors: aaronfranke akien-mga


text:
Improve support for 64-bit types on Android.

Fixes #67193.
This update mappings in the JNI functions to better support 64-bit integers, and adds support for 64-bit float arrays.
The code is mostly reused from 32-bit types.

I have now added support for 64-bit integer arrays and updated get_jni_type and get_jni_sig.
I've also added some lines that should allow for more flexible conversions involving long integers.
I really appreciate your reviewing this, because I unfortunately don't have access to an Android device, so I'm basically going blind 🙂.

Rebased this PR following #67581, which contained some of the changes included here.

@m4gr3d Sorry to bump, but would you mind taking a look at this when you have time?

Thanks!

authors: TechnoPorg akien-mga


text:
Improve tooltip for CanvasLayer.layer

For #69716
Improve description to make it clear that layers with a lower value are drawn behind layers with higher values
Production edit: Closes https://github.com/godotengine/godot/issues/69716

Thanks!

authors: YuriSizov hackenshaw


text:
Added rad2vec and deg2vec functions

Implements #7263

BTW, you could have used a force-push.. but, no big problem with this approach either :smiley:

Code looks good, but the following code:
print(rad2deg(deg2vec(20).angle()))
print(rad2deg(deg2vec(70).angle()))

Results in:
70
20

Which is obviously wrong (values are swapped). Seems like something is wrong with the axes to me, or with Vector2.angle...

Looks like Vector2.angle is implemented backwards.
32    real_t Vector2::angle() const {
33
34    	return Math::atan2(x,y);
35    }
Math::atan2 is takes the y argument first, and the x argument second. Documentation.

Yes, see #2241 and #6330.

Tested this - Works like a charm! :)

Conversion between degrees and a general 2D vector doesn't make any sense to me, in mathematical terms: the norm of a 2D vector isn't necessarily equal to 1. You can't convert between two degrees of freedom (Vector2) and one degree of freedom (an angle) in a meaningful way.
I would rather propose adding functions that converts between polar and cartesian coordinates in 2D: (x,y) <-> (r,θ) where θ is in radians. The API could be something like cartesian_to_polar(x,y) and polar_to_cartesian(r,θ), each returning a vector.

The particular use case mentioned in #7263 is Vector2(1,0).rotated(angle) BTW.
Issues regarding 2D rotations and atan2 are fixed in the current tip.

I agree with @tagcup, I prefer we teach people about polar coordinates and introduce proper constructors or converters than to add an implementation-specific rad2vec API (which would be actually angle_to_unit_vector_with_that_angle, which is only one of the many vectors that share that same angle).

Bump. This pull is stuck.
@tagcup ( @akien-mga ) Would it suffice if we rename the functions to
cartesian2polar(x,y)/polar2cartesian(angle, t = 1) or
unit_vector_with_angle(angle)/vector.angle()

Sure (assuming the implementation is also changed to be that way). cartesian_to_polar and polar_to_cartesian reads better to me. I personally don't think using 133t speak in the API is a good idea, I'd be in favor of renaming such functions that already exist.

Closing based on the above discussion. I repurposed #7263 to be about implementing cartesian to polar conversion functions.

authors: Tybobobo akien-mga bojidar-bg jxfaith tagcup


text:
Add String.strip_bbcode() and String.bbcode_escape() BBCode methods

String.strip_bbcode() can be used to display text from a RichTextLabel in places that don't support rich formatting.
String.bbcode_escape() can be used to prevent BBCode injection in RichTextLabels that display user input.

This closes https://github.com/godotengine/godot-proposals/issues/5056. See also https://github.com/godotengine/godot/pull/65839, which provides an alternative implementation that only has String.strip_bbcode() implemented.

Preview
From top to bottom: unescaped, stripped, escaped

@Calinou Just tested and using the same format as your example it seems like strip_bbcode() strips both BBCode and non-BBCode tags.

@markdibarry See https://github.com/godotengine/godot-proposals/issues/5056#issuecomment-1202460982:

It should likely strip every [tag]-like substring in the source string (and the closing pair, if present), without relying on Godot's own implementation of BBCode and the tags that we have defined.

It removes all tags, whether they exist or not. It's a String method, not a RichTextLabel method, so it can't know which tags exist and which don't. Just like to_pascal_case() method uses general rules and does not recognize specific abbreviations.

@dalexeev Ah, I see now. I think it would be misleading to call it strip_bbcode() then, since it strips all tags whether or not it contains BBCode. I think strip_tags() would be a more appropriate name, since it can't verify anything to do with BBCode, and this also would prevent a name conflict if in the future someone implemented a proper way to strip BBCode from text. Though, that would probably be part of the text server or RTL... though I can't remember if Godot's BBCode tag logic is tightly coupled to that node. That would probably need changed in the future as well, if so.

I think it is fine as is. bbcode refers to BBCode syntax, not a list of specific codes. Any [tag] is a BBCode tag, any <tag> is a HTML tag. Just like is_valid_identifier() method only analyzes the string itself, but does not answer the question whether this identifier is declared in your script or not (and does not even answer the question whether the string is a keyword like if, var, etc.).

I disagree, but you're the decision makers. Just pointing out that this method conveys that it removes BBCode, but instead, it removes BBCode, BBCode-similar tags, and any content that happens to be within brackets.
I, and two other devs I know who have established dialog languages for Godot, use tags for our language and also support BBCode. The problem, as we've discussed, is that in order to assign text, the current workflow is:

Assign the entire string with BBCode and custom tags to the RTL text field.
Call get_parsed_text() to get the text without BBCode
Take that result and parse it for custom tags.
Rebuild the string adding the BBCode as you go, comparing the two strings.
Reassign the new text.

This interested me (and I'm sure others), because I assumed it actually stripped BBCode, preventing us from having to assign the text field twice. It doesn't, so it's not relevant to our cases, but since it also removes all tags, even if they're not in BBCode syntax format, I don't see how it'd would be reliable to use in any scenario.

Currently would heavily appreciate these methods, as it would prevent users from writing BBCode in a chat system I'm currently experimenting with.

@Calinou Any chance you can name it strip_tags() and tag_escape() instead since this doesn't check for bbcode? Pretty, pretty please? I'd be so grateful.

@Calinou Any chance you can name it strip_tags() and tag_escape() instead since this doesn't check for bbcode? Pretty, pretty please? I'd be so grateful.

We already use bbcode in RichTextLabel's properties, while tag is a generic term that doesn't refer to any specific syntax.

@Calinou I know, I know. It sounds totally pedantic, I just knew I'd regret it if I didn't throw my reasoning out there before it's too late, so I want to be annoying and express my concerns while I still can:
You're absolutely right that there's no official standard for BBCode. Almost every program out there that uses the term BBCode uses their own implementation/flavor, but there is a current established implementation that we use in Godot, and this method doesn't support it. I'm not invested in the word "tag" at all, it just seems like the closest word to what it actually does without being misleading, and I couldn't think of something better. It makes more sense in my head to use a generic word for this method that works in a generic way, rather than use a word that means something else specific in Godot, since that can cause a name conflict in the near future. String.strip_bbcode() seems to work like a method called Node.remove_sprite2ds() that removes all child Node2Ds.
More selfishly: My projects (and a few others I know) use dialog systems that get bogged down with heavy string manipulation required to strip RichTextLabel's implementation of BBCode, since Godot doesn't currently provide an avenue to do so without a bunch of hacks. I've spoken to a few others that maintain dialog systems and we generally all employ the same hack to strip BBCode, because stripping all text between brackets would break our systems. Even though we wouldn't be able to use this method, I would love it if someday a performant way to strip BBCode was provided, so I am invested in an eventual RichTextLabel.strip_bbcode() being available.
If Godot's implementation of BBCode and the BBCode that this method supports are incompatible, then that raises a lot of questions. In the case of a method to strip Godot's implementation of BBCode was eventually exposed, what would happen with these methods? I'd think it'd be confusing for many to have two methods that take strings that are named the same thing, but do something different. Would we rename the string.strip_bbcode()? What would we rename it to? If so, why not now? Would we have to have a different name for the RichTextLabel version? What would that be named? RichTextLabel.strip_godot_bbcode()?

If you want a specialized and reasonably performant workaround then I'd say just to use regex, as long as the documentation clarifies it isn't selective I don't see any reason to bog this down or rename it, naming it anything else would be confusing IMO, the only other name I'd say is non-confusing would be something like strip_square_bracket_tags or something verbose, because otherwise the assumption to me would be <tag> tags

That's all a fair point to be made. Perhaps it is best to delegate this method to RichTextLabel itself to have more control over tags. However, there have also been occasions where the editor may need to strip generic BBCode from a text before displaying it, too. Perhaps there should be a static method for it, too...

@markdibarry Formally, [unknown_tag] is also a BBCode (can be BBCode), so its removal is correct. Just RichTextLabel does not report unknown tags and invalid BBCode markup, but displays them as is.
If you are dealing with user input, then you should use escape_bbcode(). If you are dealing with valid BBCode markup, then you can use strip_bbcode() to display text where BBCode is not supported. If you are dealing with invalid BBCode markup, then strip_bbcode() may not be suitable, and you will need to implement the necessary functionality yourself.
If we want to achieve more accurate (but more verbose) naming, then I would suggest the escape_bbcode_markup() and strip_bbcode_tags(). strip_tags() is too general.

I agree that there is good use for this method, and I don't want to hold it up too much, I just wanted to share my thoughts, since I can already see problems in the future, but only with name conflicts. Some background:
In RichTextLabel, when you use valid Godot BBCode tags, it'll be applied, otherwise, it displays it as normal text.
Ex:
This word is [color="green"]green[/color] and this does [foo="bar"]nothing[/foo].
would display as
This word is green and this does [foo="bar"]nothing[/foo].
In most of the dialog systems for Godot (including mine), the built in dialog code is between brackets ([mood="happy"], [GetName("John")], or [jumpTo="Section 3"]), but it also supports Godot's RichTextLabel implementation of BBCode, so the system needs to know the difference between valid Godot BBCode tags, valid dialog code tags, and normal text.
The only current avenue to get the text with valid Godot BBCode tags removed, is to assign it to the text property first and call get_parsed_text() on the node. This needs done every time any text changes:
# in RichTextLabel node
text = fullText
var parsedText = get_parsed_text()
# for dialog parsing
var dialogTags = get_dialog_tags(fullText)
text = reconstruct_text(fullText, parsedText, dialogTags)

This proposed method would remove valid Godot BBCode, valid dialog code, and any intentional non-code bracket surrounded text. So, of course, it can't be used for anything that needs awareness of Godot's implementation of BBCode, since it works on a very general level of anything that is between brackets. However, it'd be very helpful to someday have a static method that parses Godot's implementation of BBCode from a string without needing to update the RichTextLabel's text twice. The question still is, if we were to implement this, would both methods be called strip_bbcode() even though "BBCode" means two different things in these contexts?

The only current avenue to get the text with valid Godot BBCode tags removed, is to assign it to the text property first and call get_parsed_text() on the node.

Note that you are not required to use get_parsed_text(). You can implement a BBCode parser that takes into account both standard BBCode and custom pseudo-tags. For example, this is how Editor Help and the doc comment parser in GDScript are made. Also, to simplify things, you can use delimiters other than brackets.
String.strip_bbcode() is designed to deterministically process strings, regardless of the specific RichTextLabel instance, its settings, and pseudo-tags. For example, this method can be used with print_rich().
As for RichTextLabel and advanced scenarios like dialog systems, I'm not sure which method we might need in the future. There may be strip_bbcode() which you described. There may be some kind of parse_bbcode() (although the BBCode parser is not difficult to implement in GDScript). There are also proposals to add support for alternative markup such as Markdown.

The question still is, if we were to implement this, would both methods be called strip_bbcode() even though "BBCode" means two different things in these contexts?

Thanks for your concerns about potential naming conflicts! I don't have a clear opinion on this issue other than the suggestion above. But I'm not sure if clarifying tags vs markup really helps. To be completely precise, the method should be named strip_bbcode_tags_and_special_chars(), but this is too verbose.
Also, see String.is_valid_identifier() and TextServer.is_valid_identifier() methods. Just like here, the methods are named the same, and "identifier" denotes different things in different contexts. However, both methods cannot be used to check whether a string is a valid GDScript identifier, since they do not take into account the _ token, keywords, built-in types, etc. I think String vs RichTextLabel would be enough to understand the difference in purpose.

But I'm not sure if clarifying tags vs markup really helps

Just to reiterate, I have no need for the word "tag" to be used, it's just the closest general word I could think up that doesn't have a double meaning in the code base. I'm more on the "not bbcode" team than the "yes tag" team. :joy:

To be completely precise, the method should be named strip_bbcode_tags_and_special_chars(), but this is too verbose.

I doubt this would be a widely used method, but I also totally agree about keeping verbosity to a minimum.

I think String vs RichTextLabel would be enough to understand the difference in purpose.

I also like to think my code is self-documenting, but I don't think most users know that RichTextLabel and String have two different implementations of the BBCode concept; The first pulling from an internal list of valid tags, and the second meaning anything between brackets. To be honest, it sounds obvious since I assume you and I have been familiar for so long, but I originally thought there was a standard, and it was a surprise to find out that it's still kinda Calvinball out there as far as BBCode goes. :sweat:
I think if it's really important that they're named the same, some detailed documentation on the method's limitations may help alleviate some potential confusion among the community. I think at least just a note that says "Does not consider any specific BBCode implementation when parsing." or something similar would help.

Also, to simplify things, you can use delimiters other than brackets.

I can't speak for the other dialog system maintainers, but I personally think it's definitely a double-edged sword! You're right that it does make it tricky to parse, but the benefits make it worth it. Using a unified syntax can be very helpful to users looking to adopt, without having them needing to memorize a ton out of the gate. Especially when those who are most likely to use the dialog system aren't programmers. Plus, the obvious, it wouldn't be a great solution to completely rewrite the system and make all users upgrade and relearn a new syntax.

You can implement a BBCode parser that takes into account both standard BBCode and custom pseudo-tags.

Okay, this is something new to me! You've got me very interested! How do you access the internal tag list RichTextLabel supports? I'd be totally down to spend some time making a custom parser, and I know a ton of people in the community who'd be interested in having an optimized solution. I had originally planned to just copy the internal tag list into my project, but I see that the list does get changed every so often for what is/isn't supported, so that was a no-go.

Okay, this is something new to me! You've got me very interested! How do you access the internal tag list RichTextLabel supports? I'd be totally down to spend some time making a custom parser, and I know a ton of people in the community who'd be interested in having an optimized solution. I had originally planned to just copy the internal tag list into my project, but I see that the list does get changed every so often for what is/isn't supported, so that was a no-go.

This is not relevant to this PR, but I made a gist (if you have any questions, please leave comments there, not in this PR). It is not possible to obtain the tag list via Godot API, see source code instead. However, you probably won't need to get the full list, you can only process custom pseudo-tags and individual tags, such as [url] (option 2 in the gist). If you don't need to validate user input, you probably don't need a parser, you can just use String.replace() or at most regular expressions.

It is not possible to obtain the tag list via Godot API

Ah that's a shame. Your gist looks like what I came up with a few years ago, but this would require constant maintenance, so it wouldn't be of any use. Thanks anyway! I appreciate you trying!
Seeing as though I'm the minority on the naming ambiguity, as long as there's a small note in the method description about its limitations or intended use, I won't push it any further. It's enough to point users to properly read the docs if they're confused. 😅

Hey, is this going to be added? Would be useful to have a neat built-in function.

Hey, is this going to be added? Would be useful to have a neat built-in function.

4.3 is in feature freeze now, so this will probably not be merged in that version. That said, it should be straightforward to implement the functions from this PR in GDScript so you can use them in your project.

See this comment from the original proposal if you want a work-around in the meantime.

With time thinking about it, I do think this method belongs inside RichTextLabel. Whether it should be static or directly tied to a RichTextLabel object (hence it'd be able to distinguish valid BBcode tags), I am not sure.

With time thinking about it, I do think this method belongs inside RichTextLabel. Whether it should be static or directly tied to a RichTextLabel object (hence it'd be able to distinguish valid BBcode tags), I am not sure.

Here are code examples of both implementations.
RichTextLabel:

# node reference
@onready var label = $RichTextLabel
var stripped_text = label.strip_bbcode("[center]Text Here")

# or, class reference
var stripped_text = RichTextLabel.strip_bbcode("[center]Text Here")

String:

var stripped_text = "[center]Text Here".strip_bbcode()

Personally I prefer how the code reads when it is under the String class. It also functions like other immutable String methods, so the behavior is clearer from the outset. Implementing it under RichTextLabel would imply (to me at least) that the method would render or otherwise modify the text in some way, when all it does is return a modified String.

authors: AThousandShips Calinou Jael-G Mickeon dalexeev markdibarry tumblewed


text:
[macOS] Fix borderless mode on macOS 13.6+.

Seems like the behavior changed in macOS 13.6 and 14.x.
Fixes https://github.com/godotengine/godot/issues/82349

Does this need a check for older macOS versions, or does this fix the borderless behavior across all versions?

Does this need a check for older macOS versions, or does this fix the borderless behavior across all versions?

Version check is unnecessary, it should work on older versions as well.

Thanks!

Cherry-picked for 4.1.3.

authors: BrixsterPlanet YuriSizov akien-mga bruvzg


text:
N1cko m99 patch 1

Please do not submit testing pull requests to this repository. Submit them against your repository's fork instead (or create another source repository if you can't use a fork for some reason).


text:
Subemitter emission at end now correctly emits at end

Particles were previously arbitrarily emitting "at end" at 95% of lifetime. Probably worked ok for most particles, but for long living particles it's really really visible.

Remember to capitalize your commit messages ;)

Thanks!

authors: QbieShay akien-mga


text:
Fix check in Object._ValidateProperty example

The GDScript version above makes the number property read only whenever is_number_editable is false.
func _validate_property(property: Dictionary):
	if property.name == "number" and not is_number_editable:
		property.usage |= PROPERTY_USAGE_READ_ONLY

The C# version is similar, but omits the negation, so the Number property is made read only whenever is_number_editable is true.
This adds the negation to the C# example, making it match the GDScript example.

Thanks!

authors: akien-mga esainane


text:
confusing docstrings

The docstring for stan() says that the data and ProjDir arguments are required, but in the source they're defined as optional positional arguments.  Also, the way they're described is confusing since you don't call the method with the argument name (while you do for the keyword arguments, which are described similarly).
I think it would help a lot to have example syntax for all of them.  If I get time I'll open a PR but wanted to point this out here since it came up on slack.

Thank you for raising the issue here, as I’m not on slack. I’m certainly open to suggestions on how to improve the docs and/or method call.
As the call to stan() does not use multiple dispatch, this could be changed in a next major release.
Does the current set of examples help? E.g. Binormal is an example that does not need input data, all other examples do. Although it is possible to run from the current working directory, I think in most cases it is probably better to set it explicitly.

Thank you Dave for the PR. The PR made it much clearer what you were asking for and I felt it is a major improvement! Merging it was therefor a no-brainer. Please do not hesitate to suggest further updates, even minor ones.

My pleasure! Thanks for merging quickly.

authors: goedman kleinschmidt


text:
Support for dynamic gosec matrix

Dynamically generate a matrix for testing with gosec

CI still not working, pending https://github.com/tchupp/actions-detect-directory-changes/issues/18

repo: gofiber/storage


text:
os/glog/glog_logger_rotate.go:The time for slicing should be based on the file creation time rather than the update time

Go version
go1.22.3 darwin/amd64
GoFrame version
v2.7.1
Can this bug be reproduced with the latest release?
Option Yes
What did you do?
日志配置了按时间切片。配置如下：
rotateSize: 100000000
rotateExpire: "24h"
rotateBackupLimit: 30
rotateBackupExpire: "720h"
rotateBackupCompress: 0
rotateCheckInterval: "1h"

What did you see happen?
比如昨天12点启动程序，生成了日志文件log/access.log,持续到今天13点时，也没有生成log/access.log的切片。
What did you expect to see?
应该按文件的创建时间来判断是否生成切片，而不是修改时间。
框架的相关逻辑如下：
github.com/gogf/gf/v2@v2.7.1/os/glog/glog_logger_rotate.go:163
			mtime = gfile.MTime(file)
			subDuration = now.Sub(mtime)
			if subDuration > l.config.RotateExpire {
				func() {
					memoryLockFileKey := memoryLockPrefixForPrintingToFile + file
					if !gmlock.TryLock(memoryLockFileKey) {
						return
					}
					defer gmlock.Unlock(memoryLockFileKey)
					expireRotated = true
					intlog.Printf(
						ctx,
						`%v - %v = %v > %v, rotation expire logging file: %s`,
						now, mtime, subDuration, l.config.RotateExpire, file,
					)
					if err = l.doRotateFile(ctx, file); err != nil {
						intlog.Errorf(ctx, `%+v`, err)
					}
				}()
			}

其中获取文件时间这块mtime = gfile.MTime(file)实际取到的文件修改时间。

只能通过给日志文件名添加{Y-m-d}日期配置来让按时间切片生效，因为linux系统文件没有创建时间。

无需处理

repo: gogf/gf


text:
OpenTelemetry V2.8 improvement suggestions

Is your feature request related to a problem?
Option No
Describe the solution you'd like
目前工作中存在以下痛点，V2.8 中不支持。

可以定制一个参数，开启实现SQL记录
可以定制一个参数，开启实现请求参数的记录
可以定制一个参数，开启实现返回参数的记录
需要增加HTTP请求方法的记录 GET/PUT/POST/DELETE

Describe alternatives you've considered
暂无
Additional
No response

支持一下

有没OpenTelemetry使用的例子,目前比较难上手,按照官方demo搭不起来

Good proposal, we can consider it together @houseme .

有没OpenTelemetry使用的例子,目前比较难上手,按照官方demo搭不起来

是客户端还是服务端呢？服务端文档：https://www.jaegertracing.io/docs/1.63/getting-started/
客服端：https://github.com/gogf/gf/tree/master/example/trace

Good proposal, we can consider it together @houseme .

这一块的配置可以改成独立处理，
1、http 请求的在 server 配置，
2、数据库的在database配置中，
3、针对：‘需要增加HTTP请求方法的记录 GET/PUT/POST/DELETE’建议用户自己封装中间价处理，可以塞到最终response处理的中间价MiddlewareHandlerResponse中。
欢迎大家一起共建。

authors: gqcn houseme macnie wisonlau yiqiniu


text:
gf-cli Bug of generating Dao file of MSSQL

[mssql中的TableFields产生的field的key是小写的，gdb.TableField.Name是大写的导致生成dao文件失败 bug #1780

Codecov Report

Merging #1818 (89f77a2) into develop (583d576) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1818      +/-   ##
===========================================
- Coverage    71.19%   71.08%   -0.11%
===========================================
  Files          463      465       +2
  Lines        43534    43891     +357
===========================================
+ Hits         30993    31201     +208
- Misses       10564    10678     +114
- Partials      1977     2012      +35

Flag
Coverage Δ

go-1.15
71.16% <ø> (-0.01%)
:arrow_down:

go-1.16
71.14% <ø> (-0.04%)
:arrow_down:

go-1.17
71.07% <ø> (-0.10%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

database/gdb/gdb.go
60.33% <0.00%> (-12.21%)
:arrow_down:

database/gdb/gdb_core_utility.go
53.43% <0.00%> (-3.48%)
:arrow_down:

database/gdb/gdb_core.go
68.27% <0.00%> (-2.64%)
:arrow_down:

os/glog/glog_logger_rotate.go
65.85% <0.00%> (-1.47%)
:arrow_down:

os/gcron/gcron_z_unit_schedule.go
67.09% <0.00%> (-0.43%)
:arrow_down:

os/gcron/gcron_schedule.go
67.09% <0.00%> (ø)

database/gdb/gdb_default_driver.go
7.69% <0.00%> (ø)

container/gset/gset_str_set.go
93.31% <0.00%> (+2.62%)
:arrow_up:

net/gtcp/gtcp_pool.go
38.63% <0.00%> (+4.54%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 583d576...89f77a2. Read the comment docs.


text:
gogs_v0.9.0_linux_amd64 升级 gogs_v0.9.13_linux_amd64 失败.

We DO NOT take questions or config/deploy problems on GitHub, please use our forum: https://discuss.gogs.io
Please take a moment to search that an issue doesn't already exist.
For bug reports, please give the relevant info:

Gogs version (or commit ref): 0.9.13.0318
Git version: 2.1.4
Operating system: debian8
Database:

[ ] PostgreSQL
[x] MySQL(MariaDB10.0.23)
[ ] SQLite

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

[ ] Yes (provide example URL)
[x] No
[ ] Not relevant

Log gist:

Description
升级覆盖相关文件后，在服务器启动，提示警告，然后就不能访问了.
错误日志:
2016/04/07 10:09:04 [I] Gogs: Go Git Service 0.9.13.0318
2016/04/07 10:09:04 [I] Log Mode: File(Trace)
2016/04/07 10:09:04 [I] Cache Service Enabled
2016/04/07 10:09:04 [I] Session Service Enabled
2016/04/07 10:09:04 [I] Git Version: 2.1.4
2016/04/07 10:09:04 [I] Migration: convert date to unix timestamp
2016/04/07 10:09:04 [I] Converting table: action
2016/04/07 10:09:04 [T] Table [action]: offset: 0, beans: 100
2016/04/07 10:09:09 [T] Table [action]: offset: 100, beans: 100
2016/04/07 10:09:15 [T] Table [action]: offset: 200, beans: 100
2016/04/07 10:09:20 [T] Table [action]: offset: 300, beans: 100
2016/04/07 10:09:24 [T] Table [action]: offset: 400, beans: 100

你这个日志就是还没升级完成，正在做 migrate，我这个升级等了好久的。。。

@terrywh has said what needs to be said...

authors: Unknwon terrywh yzdel200

repo: gogits/gogs


text:
v0.9.46 missing release binaries

Some binaries including linux_amd64 are missing at https://github.com/gogits/gogs/releases/tag/v0.9.46
Thanks for the release! :tada:

I don't have enough machines and no good internet to setup VMs yet, will upload ASAP.

@Unknwon What do you think about create a building platform like jenkins ?

@BurakDev I need native system to compile, so Jenkins might not help.

What about doing some build/release enginering  for the project, or allowing a devops systems engineer to work with you to clean up some of the practices.  We do that, and you can get automated builds and release, and even packaging anywhere you wanted.. or manual, and it would be perfect every time.

Vagrant can be used to build machines and could be jenkins nodes which are controlled from a jenkins master. Probably Gogs can use a build/test system the same as Syncthing does (https://docs.syncthing.net/dev/infrastructure.html).

I have found that https://drone.io combined with Gogs works extremely well for CI for git repos.

@Unknwon thanks for adding them!

Also thanks from me 👍

authors: BurakDev Unknwon Xe gramakri techzilla xor-gate


text:
Build returned "matched no package" and didn't produce output.

I am trying to build the project github.com/pseudomuto/protoc-gen-doc.  Here is my build.gradle
`
buildscript {
repositories {
repositories {
mavenLocal()
maven {
url "${artifactoryContextUrl}/libs-release"
}
maven {
url "${artifactoryContextUrl}/libs-snapshot"
}
mavenCentral()
}
}
dependencies {
classpath "com.github.blindpirate.gogradle:com.github.blindpirate.gogradle.gradle.plugin:0.7.0"
}
}
apply plugin: "com.github.blindpirate.gogradle"
group 'github.com/pseudomuto/protoc-gen-doc'
version '0.0.1'
golang {
packagePath = 'github.com/pseudomuto/protoc-gen-doc'
build {
    go "build github.com/pseudomuto/protoc-gen-doc/cmd/..."
}

}
dependencies {
golang {
build 'github.com/golang/protobuf/proto'
build 'github.com/golang/protobuf/protoc-gen-go/plugin'
}
}
`
And I am getting this output
Use project GOPATH: /Users/trpfe96/Projects/Fusion/protoc-gen-doc/.gogradle/project_gopath
warning: ignoring symlink /Users/trpfe96/Projects/Fusion/protoc-gen-doc/.gogradle/project_gopath/src/github.com/pseudomuto/protoc-gen-doc
warning: "github.com/pseudomuto/protoc-gen-doc/cmd/..." matched no packages
warning: ignoring symlink /Users/trpfe96/Projects/Fusion/protoc-gen-doc/.gogradle/project_gopath/src/github.com/pseudomuto/protoc-gen-doc
warning: "github.com/pseudomuto/protoc-gen-doc/cmd/..." matched no packages
But I can build successfully with this command
go build github.com/pseudomuto/protoc-gen-doc/cmd/...
Any idea what I did wrong?

The issue here is that the "..." does not work for this plugin.  While it is legitimate to run "go build github.com/pseudomuto/protoc-gen-doc/cmd/..." , I need to specify the exact location of the main.go when using this plugin.  So after modifying my gradle file to
golang {
packagePath = 'github.com/pseudomuto/protoc-gen-doc'
build {
    go "build github.com/pseudomuto/protoc-gen-doc/cmd/protoc-gen-doc"
}

}
The build successfully produced the output.

repo: gogradle/gogradle

license_source: bigquery


text:
Add: Statistics Command

Overview:
This PR adds stats command which shows the overall statistic information about the projects and repositories.
Screenshots:

I would  put this information into info becuase, this here so little for having its own command

While you are at info you can also add this fields in here
See: https://demo.goharbor.io/api/v2.0/systeminfo
Example
{
  "auth_mode": "db_auth",
  "banner_message": "",
  "harbor_version": "v2.10.0-6abb4eab",
  "oidc_provider_name": "",
  "primary_auth_mode": false,
  "self_registration": true
}

keep also in mind the the info is differen if the user is logged in or not.

JSON durchsuchen
auth_mode	"db_auth"
banner_message	""
current_time	"2024-05-30T22:11:59.507Z"
external_url	"https://demo.goharbor.io"
harbor_version	"v2.10.0-6abb4eab"
has_ca_root	false
notification_enable	true
oidc_provider_name	""
primary_auth_mode	false
project_creation_restriction	"everyone"
read_only	false
registry_storage_provider_name	"filesystem"
registry_url	"demo.goharbor.io"
self_registration	true

Will update it accordingly.

Info Command:

the above info command also shows the statistic as mentioned. // @Vad1mo

ready to merge!

repo: goharbor/harbor-cli


text:
Array of bundle.hash is reversed

The array of the bundle you get back after a TX is 'upside down' (reversed), which means that the address where the tokens are send to normally is bundle[0].hash, but now it is the address of the remainder tokens.
In short:
In IRI the bundle[0].hash is the address you want to send to.
In Hornet bundle[0].hash is the address your remaining tokens are sent to.

You shouldn't rely on the order at all, but on the addresses, right?

attachToTangle API call
https://github.com/iotaledger/iri/blob/1527f771f6b788c0d0f4dda473bc698db214432e/src/main/java/com/iota/iri/service/API.java#L1325
https://github.com/gohornet/hornet/blob/fbb26776f0488b28b33f3275cb1d03f7b77b46ac/plugins/webapi/pow.go#L120

	//reverse order so the first element is the tail tx
	if len(txs) > 1 {
		for i, j := 0, len(txs)-1; i < j; i, j = i+1, j-1 {
			txs[i], txs[j] = txs[j], txs[i]
		}
	}

	powedTxTrytes := transaction.MustTransactionsToTrytes(txs)

Don't know if there is a better way, but you could do it that way if len(txs) > 1 {} could also be removed

authors: HarmvandenBrink Thoralf-M muXxer

repo: gohornet/hornet


text:
Add a .Markdownify on Page

Find a better name, but this is related to #5975 -- the markdownify has no page context so it just renders the content as "Blackfriday markdown", which is not ... perfect if in a shortcode and the surrounding content is Asciidoc.

To me markdownify is a misleading function name. All other -ify and -ize functions do what the name says but markdownify does the opposite (maybe this matter has been up for discussion before).

I always understood it like Markdown is mark-down because it is stripped from the mark-up added in HTML (which stands for Hyperthext-Markup-Language). Given that, the verb to transform Markdown to HTML... shouldn't it be... «markup»?

@cluck you are right.

Continuing from #6545. Ideally we would need to choose which page to use as a Page context on any string rendered with markdownify and when in need for this we could use .Markdowify but, we could also introduce a second optional argument to markdownify for a Page context.
We should test that second argument type before using it as the Page context though because a lot of people are using Go Pipes:
{{ .Params.body | markdownify }}

Which is fine as long as the function ony take one argument, but once it takes two, that .Params.body will be considered the last arg and be used as the Page context...

This is fixed in https://github.com/gohugoio/hugo/commit/e625088ef5a970388ad50e464e87db56b358dac4 method is .RenderString.

authors: bep cluck larzza regisphilibert

repo: gohugoio/hugo


text:
Update README.md

Centered the logo because it looks visually better?

 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.

Looks like some of your markup is invalid. You can't mix p tags and markdown quite like that.

authors: CLAassistant EOussama RealOrangeOne


text:
Add plugin support

Plugins add support for sending request metrics, and logging #2

@sohamkamani A major change to the structure of the API has landed on master, can you plz rebase with master and inculcate the new changes. Thanks

@rShetty fixed

@sohamkamani You want to look at fixing merge conflicts, will have a look and merge it

@rShetty Yes! Will look at the conflicts and let you know!

Coverage decreased (-14.02%) to 73.214% when pulling 0c0c294908a5bab28f5b344e89625b16ddd19e6e on plugins into ef69e0bb0cbf72e832303efe6362855811e8cc89 on master.

@sohamkamani Coverage 🤔

ya its a bit misleading since I added the mock to the main package.. fixing this

@rShetty is this ok to be merged?

@sohamkamani Will review this, sorry for the delay, caught up with multiple things. Will review so soon. thanks

Has there been any progress on this? I was just about to wire in httptrace, but I would rather use this if possible.

Hello, is there any progress on this? Is this repo still being maintained?

authors: coveralls jjafuller rShetty sohamkamani unmultimedio

repo: gojek/heimdall


text:
CRLF in golden files

Git on PC's has an autoCRLF feature - automatically converting to CRLF line endings when populating files, but storing those files with Unix-style LF-only endings.  This is an issue with the golden-file test pattern - the golden files on a PC with autoCRLF on will have CRLF endings, but the internal generation being tested only has LF endings, and the tests fail.  If we ask PC users to drop autoCRLF, we'll have random files being checked in with CRLF line endings.  Maybe this is ok?  Would the CI tests catch this?
I am in the process of adding the golden file pattern to the integration tests.  Should I look to add a CRLF-stripping function to the file read function in the test helper?

A repo level .gitattributes file can override autocrlf, either just for a few files or the entire repo. We could add a config file and tell windows to not dork up the golden files.

If playing with Windows line endings isn't your cup of tea, I can help test this out and submit a PR with the testdata/ dirs excluded from being rewritten.

If there is a git configuration that can be checked into the repo to fix the problem, that would indeed seem better than a code fix. I have a pipe running, but it feels like a hack.

I'll work on that today.

repo: golang/dep


text:
gccgo: crash on reference to undefined name

package main
func crash() {
        xpc := pc
}

gccgo (GCC) 5.0.0 20150331 (experimental) on ppc64le
error: reference to undefined name ‘pc’
  xpc := pc

go1: internal compiler error: in set_type, at go/gofrontend/expressions.cc:15628
0x101d2b0f Numeric_constant::set_type(Type*, bool, Location)
        ../../gcc/go/gofrontend/expressions.cc:15628
0x101d45a7 Const_expression::do_numeric_constant_value(Numeric_constant*) const
        ../../gcc/go/gofrontend/expressions.cc:2691
0x101d5493 Expression::numeric_constant_value(Numeric_constant*) const
        ../../gcc/go/gofrontend/expressions.h:490
0x101d5493 Binary_expression::do_lower(Gogo*, Named_object*, Statement_inserter*, int)
        ../../gcc/go/gofrontend/expressions.cc:4927
0x101fbb1f Expression::lower(Gogo*, Named_object*, Statement_inserter*, int)
        ../../gcc/go/gofrontend/expressions.h:699
0x101fbb1f Lower_parse_tree::expression(Expression**)
    ../../gcc/go/gofrontend/gogo.cc:2616
0x101b631f Expression::traverse(Expression**, Traverse*)
        ../../gcc/go/gofrontend/expressions.cc:43
0x10246a67 Statement::traverse_expression(Traverse*, Expression**)
        ../../gcc/go/gofrontend/statements.cc:83
0x10246a67 If_statement::do_traverse(Traverse*)
        ../../gcc/go/gofrontend/statements.cc:3194
0x10249677 Statement::traverse_contents(Traverse*)
        ../../gcc/go/gofrontend/statements.cc:61
0x101fc5f3 Lower_parse_tree::statement(Block*, unsigned long*, Statement*)
    ../../gcc/go/gofrontend/gogo.cc:2570
0x102495cb Statement::traverse(Block*, unsigned long*, Traverse*)
    ../../gcc/go/gofrontend/statements.cc:43
0x10200c7b Block::traverse(Traverse*)
        ../../gcc/go/gofrontend/gogo.cc:5578
0x102578fb Block_statement::do_traverse(Traverse*)
        ../../gcc/go/gofrontend/statements.cc:1814
0x10249603 Statement::traverse(Block*, unsigned long*, Traverse*)
        ../../gcc/go/gofrontend/statements.cc:53
0x10200c7b Block::traverse(Traverse*)
        ../../gcc/go/gofrontend/gogo.cc:5578
0x10200fcf Function::traverse(Traverse*)
        ../../gcc/go/gofrontend/gogo.cc:4739
0x10209b63 Lower_parse_tree::function(Named_object*)
        ../../gcc/go/gofrontend/gogo.cc:2547
0x10204a7f Bindings::traverse(Traverse*, bool)
        ../../gcc/go/gofrontend/gogo.cc:7353
0x10204c3b Gogo::traverse(Traverse*)
        ../../gcc/go/gofrontend/gogo.cc:2367
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

The gccgo-5 (Debian 5-20150226-1) 5.0.0 20150226 (experimental) [trunk revision 220999]
test.go:3:16: error: reference to undefined name ‘pc’
         xpc := pc

commit 3897c269d567836ca49848322062d1c8c43f4177
Author: gccadmin gccadmin@138bc75d-0d04-0410-961f-82ee72b054a4
Date:   Tue Mar 31 00:16:11 2015 +0000

I did a full bootstrap of GCC on a powerpc64le machine I have here, from gcc trunk SVN revision 221802.  I can not recreate the problem.  I just get the error message and no compiler crash.
I don't plan to look at this further, but let me know if you track it down.

ok. I can do that next time I upgrade. I dont need this, it was just found with my permutation tool.  I wonder if we have a test suite for regression testing?

gccgo is using mostly the same testsuite as gc's.
Use "make check-go" to run it.

In fact the gccgo testsuite is an exact copy, though it has a different driver.
But I'm not all that interested in adding crash-on-invalid bugs to the testsuite.  They should be fixed, of course, but I'd rather only add invalid code to the testsuite to test error messages and to make sure that the compiler correctly rejects invalid code.  I don't think that compiler crashes on invalid code are worth growing the testsuite for.

fine with me, lets just leave this one for now until someone finds time for it. I will focus on valid code now. just trying to help.

I appreciate the help.  I do want to fix the problem if we can identify it.  I only meant to explain that I don't think that crash-on-invalid tests need to be added to the testsuite.

Sorry, didn't mean to close this.

As far as I can tell, this is fixed.  Closing.

authors: h4ck3rm1k3 ianlancetaylor minux

repo: golang/go


text:
pprof: weird allocation counts after reading a file of a certain minimum size

Here is a gist containing a program that does the following:

Set runtime.MemProfileRate to 1.
Read a flag-controlled number of zeroes from /dev/zero using ioutil.ReadAll and io.LimitReader.
Call io.MultiWriter 1024 times in a loop, encouraging it to make allocations.
Write out a heap profile to /tmp/mem.pprof.

When I run the program with the flag that tells it to read 2096128 bytes before calling io.MultiWriter, I see the expected number of calls in the profile:
% go build main.go && ./main --size 2096128 && grep -B 1 MultiWriter /tmp/mem.pprof
1024: 32768 [1024: 32768] @ 0x43aaf5 0x45dd38 0x400f60 0x4010a1 0x42a3ff 0x456331
#       0x45dd38        io.MultiWriter+0x48     /usr/local/google/home/jacobsa/clients/go/src/io/multi.go:58
--
996: 31872 [1024: 32768] @ 0x45dda3 0x400f60 0x4010a1 0x42a3ff 0x456331
#       0x45dda3        io.MultiWriter+0xb3     /usr/local/google/home/jacobsa/clients/go/src/io/multi.go:60

However, with any value above this I get unreliable results (these change from run to run, but are never the full count and are often zero):
% go build main.go && ./main --size 2096129 && grep -B 1 MultiWriter /tmp/mem.pprof
0: 0 [0: 0] @ 0x45dda3 0x400f60 0x4010a1 0x42a3ff 0x456331
#       0x45dda3        io.MultiWriter+0xb3     /usr/local/google/home/jacobsa/clients/go/src/io/multi.go:60
--
0: 0 [0: 0] @ 0x43aaf5 0x45dd38 0x400f60 0x4010a1 0x42a3ff 0x456331
#       0x45dd38        io.MultiWriter+0x48     /usr/local/google/home/jacobsa/clients/go/src/io/multi.go:58

This shows up as nonsense callgraphs in pprof, containing lots of runtime frames and the wrong counts for the functions I'm interested in. Compare this one to this one. Each is from an invocation of pprof that looks like:
go tool pprof -alloc_objects ~/tmp/59f9bbfa096fd3c595dc/main /tmp/mem.pprof

I have been unable to simplify the test case further. In particular, merely allocating a slice of bytes of the given size isn't good enough; it seems I have to open and read a file. I've also seen the problem when not setting runtime.MemProfileRate to a non-default value, but it is harder to reproduce.
go version:
go version devel +f2662f2 Tue Jun 23 03:18:52 2015 +0000 linux/amd64

Marking this Go1.5 so we at least understand what's happening.

The heap profile reports a snapshot as of the most recent garbage collection. It omits more recent allocations, because otherwise the recent garbage in the profile can drown out the older live data. What's going on in your program is that the size of the read affects whether a garbage collection happens after the multiwriters are created. If so, they appear in the profile; if not, they don't.
If you want to see a snapshot of live data as of the writing of the profile, you need to call runtime.GC before writing out the profile. If you only care about allocation counts and not what is live, then you can also use GOGC=off. When the GC is off, the profile reports all allocations (since there's no "last GC").
So this is working as intended, if possibly mysteriously. I will update the runtime/pprof doc comment to make this clear.

Thanks so much for the explanation. Sticking something similar in the docs sounds good to me.

Oh, to clarify: when you say "see a snapshot of live data", do you mean to say only objects that are still live? Or can I call runtime.GC just before writing the file to see historical allocation counts, even for objects that have long since been collected?

@jacobsa Each profile line gives n: m [N: M] for a particular allocation stack, where n: m is currently allocated (live) objects, and N: M is all objects. pprof --inuse_xxx uses n: m and pprof --alloc_xxx uses N: M. What I mean by "a snapshot of live data" is really "a snapshot of right now", because those are updated only at each GC.

Got it, thanks for confirming.

authors: bradfitz jacobsa rsc


text:
proposal: different octal base literal representation

Shortly
Proposal is to add an alternative syntax for representing octal literals and deprecate the current syntax in Go 2+. Deprecation is not necessary, but is personally perceived to be inline with Go's idioms
Current syntax: octal_lit   = "0" { octal_digit } .
Proposed change: octal_lit   = "0o" { octal_digit } .
Background
The idea is directly borrowed from CoffeeScript, where the feature exists to help developers to avoid various typos in their code:

Missed x in 0x1234
Missed . after 0 in 0.1234
Missed . before 0 in .01234
Extra leading 0 in decimal literal as in 01234

Personal view on the current syntax
I think that the current syntax for octal literals is outdated and was simply copied from ancestor languages, because the problem it introduced there was not evident. This proposal may very well be rejected on the grounds of Go 1 compatibility promise assesment, but I guess it is good to keep the pressure for a change.

I don't have any opinion on this proposal, I'd just like to point out that at least 3 of those 4 points are invalid IMO.

Missed x in 0x1234

If you can miss x, what stops you missing o

Missed . after 0 in 0.1234
Missed . before 0 in .01234

I don't think most real code would even compile if you made this mistake because (presumably) you'd end up passing/assigning that float to an int and vice-versa

Extra leading 0 in decimal literal as in 01234

This one's a little less invalid, but I don't know that I see any benefit here either. I can't remember a time when I've seen such a number that wasn't octal so that kinda rules out copy-paste mistakes. And if you can type that number by mistake, what stops you from typing 11234.

@DisposaBoy thanks, I realized that I mostly agree with your comments on floating point numbers. There is one more thing though. 072.40 is a perfectly valid floating point number and is equal to 72.40. To me this is again a confusing language decision.

If you can miss x, what stops you missing o

The deprecation of a leading 0 to denote octal (floating-point?) literals will turn it into a compilation error.

072.40 is a perfectly valid floating point number and is equal to 72.40.

The compiler agrees: http://play.golang.org/p/5n0-hAM4-e

I feel like you are missing a cost/benefit discussion.  Changing the language, even in a hypothetical Go 2, carries a cost.  The benefit should be something more than personal preference.
I'm not saying that it's impossible to change the language in Go 2, if there is a Go 2.  I'm saying that the language should only be changed for good reasons that bring clear benefits.

That is true. Some cost is implied here (as with any change). It is obvious the effort should weighed against the gains. However, I am in no position to judge on that (speaking from my less than shallow experience with Go).
Any educated input on the matter is more than welcome.

If anything like this is to happen in Go 2, it should be more interesting than this. One possibility is a general way to have literals of arbitrary base, such as 3r012 where 3 is the base (radix), r is the marker, and the digits follow. This form has been used in other languages.
Regardless, I don't believe any change is worthwhile. There is no pressing need for any of this. The current triad is convenient, familiar, and within a tiny fraction of being always enough.

For the historical perspective: The leading-0 notation was not "was simply copied from ancestor languages" - this was quite a deliberate decision, with several weeks of on-and-off thinking about it. There was also discussion about a more general notation to support other number bases (2 comes to mind), or arbitrary bases. Eventually we decided to stick with the historic notation, despite its problems for several reasons:

C programmers are familiar with it, there are no surprises
there was a feeling that ignoring the leading 0 might lead to hard-to-track down errors when existing C code using the 0-notation (file access) would be ported to Go
at the time, none of us (Ken, Rob, I) had a better suggestion or felt a strong need for changing it

In retrospect, being afraid of introducing subtle errors was perhaps overly cautious, but we don't really know.
That said, I don't like you suggestion, and fwiw, both the 'o' for octal and 'b' for binary notation were also discussed in the design of Go. It's simply not enough bang for the buck.
If, and that is a big IF, there were such a change, I think it should enable arbitrary number bases with a uniform syntax (I see Rob made just the same point). I have proposed the following notation during the design phase of Go (albeit after we made the decision to stick with the 0x and leading-0 notation):
int_lit = [ radix 'x' ] { digit } .
radix = decimal_digit [ decimal_digit ] .
digit = decimal_digit | 'a' | 'A' | 'b' | 'B' | ... 'z' | 'Z' .
decimal_digit = '0' | '1' | ... '9' .
With the following semantics: A leading radix indicates the number base, any value from 2 to 36 is valid. The subsequent digits must be in the range from 0 to radix-1. For instance, 2x101 means (decimal) 5, 8x660 corresponds to our current 0660, 12x106 means (decimal) 150, etc. Additionally, we permit the radix 0 and state that radix 0 is equivalent to radix 16. This permits the common and widely recognized hexadecimal notation 0x1234 as we know it. (This is exactly what Rob suggested above, but with r replaced by x which enables backward-compatibility with our existing hex notation).
The cost of introducing this is probably small (it's done in one or 2 places in a compiler's frontend, and its trivial). But the benefit might be even smaller.

@robpike @griesemer Thank you. I agree with your reasoning. Tthis is also good to know similar thoughts have (had) been sitting in the back of your head.
This proposal is not pressing whatsoever and the least surprise principle must hold. Well, I am happy to see it implemented in a more "interesting" way, or not at all.

I am against this, for the reasons Rob and Robert already gave (which were the reasons we didn't do it in 2009).
General consensus seems against too.

As a reminder, we introduced a new process for these Go 2-related language changes in our blog post blog.golang.org/go2-here-we-come. The Go 1.13 development cycle is now over and it’s time for the final decision.
The feedback on the Go 2 number literal changes has been strongly positive, with very few negative voices. These changes modernize and harmonize Go’s number literal syntax without adding significant complexity to the language: There is now a uniform prefix notation for the three common non-decimal number bases, which matches the notation used in other modern programming languages. The introduction of hexadecimal floating-point literals addresses a pain point for people concerning themselves with numeric code. The suffix “i” may now be used with any (non-imaginary) number literal to create an imaginary constant in a uniform way. And finally, underscores may be used to split longer literals into groups of digits for improved readability.
Proposal accepted for Go 1.13. Closing because the changes have landed.
- rsc for proposal review

authors: DisposaBoy RomanSaveljev cznic griesemer ianlancetaylor robpike rsc


text:
cmd/compile: improve the help message of go tool compile -d ssa/help

I noticed it was somewhat difficult to read the ssa/help output:
  $ go tool compile -d ssa/help
  compile: GcFlag -d=ssa/<phase>/<flag>[=<value>]|[:<function_name>]
  <phase> is one of:
  check, all, build, intrinsics, early_phielim, early_copyelim
  early_deadcode, short_circuit, decompose_user, opt, zero_arg_cse
  opt_deadcode, generic_cse, phiopt, nilcheckelim, prove, loopbce
  decompose_builtin, dec, late_opt, generic_deadcode, check_bce
  writebarrier, fuse, dse, insert_resched_checks, tighten, lower
  lowered_cse, lowered_deadcode, checkLower, late_phielim, late_copyelim
  phi_tighten, late_deadcode, critical, likelyadjust, layout, schedule
  late_nilcheck, flagalloc, regalloc, stackframe, trim
  <flag> is one of on, off, debug, mem, time, test, stats, dump
  <value> defaults to 1
  <function_name> is required for "dump", specifies name of function to dump after <phase>
  Except for dump, output is directed to standard out; dump appears in a file.
  Phase "all" supports flags "time", "mem", and "dump".
  Phases "intrinsics" supports flags "on", "off", and "debug".
  Interpretation of the "debug" value depends on the phase.
  Dump files are named <phase>__<function_name>_<seq>.dump.

With a few tweaks here and there it looks more pleasing to the eye:
  $ go tool compile -d ssa/help
  compile: usage: -d=ssa/<phase>/<flag>[=<value>|<function_name>]

  <phase> is one of:

  	check, all, build, intrinsics, early_phielim, early_copyelim, early_deadcode
  	short_circuit, decompose_user, opt, zero_arg_cse, opt_deadcode, generic_cse
  	phiopt, nilcheckelim, prove, loopbce, decompose_builtin, dec, late_opt
  	generic_deadcode, check_bce, writebarrier, fuse, dse, insert_resched_checks
  	tighten, lower, lowered_cse, lowered_deadcode, checkLower, late_phielim
  	late_copyelim, phi_tighten, late_deadcode, critical, likelyadjust, layout
  	schedule, late_nilcheck, flagalloc, regalloc, stackframe, trim

  <flag> is one of on, off, debug, mem, time, test, stats, dump.
  Phase all supports flags time, mem, and dump.
  Phase intrinsics supports flags on, off, and debug.

  <value> defaults to 1.

  <function_name> is required for dump, specifies name of function to dump after <phase>.

  Except for dump, output is directed to standard out; dump appears in a file.
  Dump files are named <phase>__<function_name>_<seq>.dump.

I consider this documentation and fine for Go 1.9 if you can find a reviewer. Probably @randall77 or @josharian.

I'll review. Please consider adding a few examples of commonly used flags.

authors: bradfitz dlespiau josharian


text:
cmd/compile: treat pointer-to-go:notinheap types as scalar

In the runtime, we have the go:notinheap directive to flag types that can never appear in the Go heap. The compiler currently uses this directive to omit write barriers for *T-typed slots when T is marked go:notinheap.
However, we still mark *T-typed slots as pointers in the GC maps, worry about zero-initializing them, allocate them in the .bss/.data sections instead of .noptrbss/.noptrdata sections, etc. This issue is about determining whether we can just treat them as scalars instead; and if so, implementing that change.
See also brief discussion in CL 73412.
/cc @aclements

@mdempsky This is done after https://go-review.googlesource.com/c/go/+/249917

Thanks.

authors: cuonglm mdempsky


text:
cmd/link: some packages don't build with -buildmode=shared

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.9.5 linux/amd64
go version go1.10.1 linux/amd64
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOCACHE="/tmp/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/peter/go"
GORACE=""
GOROOT="/my/opt/go"
GOTMPDIR="/tmp"
GOTOOLDIR="/my/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build569417713=/tmp/go-build -gno-record-gcc-switches"

What did you do?
I tried to install the package github.com/pebbe/util in lgo (github.com/yunabe/lgo). The package installs and works fine as a regular, static package. But lgo tries to build shared packages, and that fails.
I don't know what is going on, but this might be a bug in Go with -buildmode=shared
You can find a detailed description here: https://github.com/yunabe/lgo/issues/57

CC @mwhudson

Hm my fix for #22998 doesn't seem to fix this. Will need to make some time to dig properly into this.

If I run
go tool compile -shared -o foo.a GOPATH/github.com/yunabe/gobug25065/src.go

then go tool nm shows
    1187 R %22%22.(*myType).A·f

If I run
go tool compile -shared -dynlink -o foo.a GOPATH/github.com/yunabe/gobug25065/src.go

(that is, add the -dynlink option) then go tool nm shows
     U %22%22.(*myType).A·f

So it seems that when using -dynlink the symbol really is undefined.

authors: ianlancetaylor mwhudson pebbe


text:
Add encoding/base58

Besides being wide spread in the cryptocurrency world, base58 is generally great for data humans interact with manually (url shorteners, password reset tokens etc).
https://en.m.wikipedia.org/wiki/Base58

Usual question: why has this to be in the standard library?
https://golang.org/doc/faq#x_in_std

Usual question: why has this to be in the standard library?

Maybe it doesn't, I'm honestly not quite sure.
Pros:

Fits well with the existing base32 and base64 packages.
Useful for web programming and generally most usecases for base32
Unlikely to be maintenance burden
Used in security sensitive contexts, where it is nice to avoid third-party code.
Shorter than base32

Cons:

Not as commonly used as base32
There are a small number of variants, but maybe only one is necessary

In terms of usecases you could list everything base32 is used for. In my case it came up in a security sensitive package where we are hesitant to add third party dependencies for trivial code, so we would probably end up creating yet another base58 package otherwise (there are about 50 open source implementations already and probably a lot more closed source ones and package-private implementations).
I expect that this is a somewhat common scenario given that base58 is a good choice for codes and ids that are checked/typed/copied by users.

authors: ALTree FlorianUekermann


text:
cmd/compile: extendslice contains n.SetLikely(false), which does nothing

extendslice contains: nifneg.SetLikely(false). But the Likely flag on Node is boolean; setting it to false has no effect at all. To make it do something, we'd need to invert the condition and set it to true. But given that the body contains a panic-style call, I suspect we already do the right thing for likeliness, and we should just delete the call to SetLikely.
This issue is a reminder to investigate and either delete or fix the SetLikely call.
cc @martisch

Thanks for filling the reminder. Makes sense to remove for Go1.14 after confirming the branch layout is correct without it. I wonder if we have any way (or should have) to explicit communicate from front to backbend that an if is not taken.

authors: josharian martisch


text:
runtime: some programs broken on Wine after removing time compatibility hacks

go version devel +03ac39ce5e Mon Sep 2 12:57:37 2019 +0000 linux/amd64

I've started seeing panics like the one below when running some of my tests via GOOS=windows go test -exec=wine:
fatal error: nanotime returning zero

goroutine 1 [running, locked to thread]:
runtime.throw(0x60406f, 0x17)
        /home/mvdan/tip/src/runtime/panic.go:774 +0x79 fp=0xc00003bf60 sp=0xc00003bf30 pc=0x4309f9
runtime.main()
        /home/mvdan/tip/src/runtime/proc.go:152 +0x350 fp=0xc00003bfe0 sp=0xc00003bf60 pc=0x432500
runtime.goexit()
        /home/mvdan/tip/src/runtime/asm_amd64.s:1375 +0x1 fp=0xc00003bfe8 sp=0xc00003bfe0 pc=0x45e2e1
exit status 2

If anyone wants to reproduce the crash, the Go package in question is https://github.com/mvdan/sh/tree/6af96bc17993a990fcd2c341c83a168a3158daa1/interp. I can provide a small reproducer if necessary, but I think the crash is pretty evident.
It looks like this was intentional as of https://go-review.googlesource.com/c/go/+/191759. The CL reads:

Wine is useful and developers will appreciate being able to debug stuff with it.

This gets a big +1 from me, of course :)

(1) has been handled for some time by Wine with the introduction of the commit entitled "ntdll: Create thread to update user_shared_data time values when necessary".

Unfortunately, the commit message didn't include a direct link to said patch or fixed bug. All I could find is this patch maintained as part of wine-staging: https://github.com/wine-staging/wine-staging/blob/a46b9ff3dcb51398cd6626f7090d8885844e1b5b/patches/ntdll-User_Shared_Data/0003-ntdll-Create-thread-to-update-user_shared_data-time-.patch
I can further prove this; the crash happens on vanilla Wine 4.15, but doesn't happen on Wine 4.15 staging.
So I don't think that Wine has handled this for some time, as the CL describes. At best, a patch has been available on wine-staging since 2017, but most users don't run wine-staging. I would personally prefer sticking to vanilla Wine, as it's more stable, and I've had no reason to use staging patches until now.
This is not an issue to report a regression and demand a revert; I understand the code in the runtime package was a hack, and that in the long term we're better off without it. However, I think we should be aware that this will break a non-trivial amount of users. And, even if Wine 4.16 shipped tomorrow with a stable fix, it would at least be a year or two before most Linux users are using that newer Wine version.
At a minimum, I think we should keep an issue like this one open, to quickly point confused users at a temporary workaround. I'm all ears on that front; I can run wine-staging for now, but that's not an ideal solution.

Afaict, this is fixed in upstream wine. Upgrade your wine. This is 1.14 material which means it's a long way off from release.

Please read my detailed report above; I tested on Wine 4.15, which was released less than three days ago. Am I already out of date? :)

Reverted the CL in question as of CL 192622; my bad for forgetting the issue link in the commit message.


text:
You are neither in a module nor in your GOPATH

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

$ go version

Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (go env)?
go env Output
$ go env

What did you do?

What did you expect to see?
What did you see instead?

There is nothing filled out in here to make this an actionable issue report.

authors: martisch youda-online


text:
doc: update install from source page after GO386 changes

The Installing Go from source page says (in the section dedicated to Env Variables):

$GO386 (for 386 only, default is auto-detected if built on either 386 or amd64, 387 otherwise)
This controls the code generated by gc to use either the 387 floating-point unit (set to 387) or SSE2 instructions (set to sse2) for floating point computations.

GO386=387: use x87 for floating point operations; should support all x86 chips (Pentium MMX or later).
GO386=sse2: use SSE2 for floating point operations; has better performance than 387, but only available on Pentium 4/Opteron/Athlon 64 or later.

The section should be updated to reflect the planned 1.16 deprecation of 387, and the softfloat support.
cc @dmitshur

Thanks for spotting this @ALTree.
@randall77 Would you like to send a CL for this? The file that needs to be updated is in x/website, here. (There's also an old copy of it in the main repo, but that needs to be deleted; I re-opened https://github.com/golang/go/issues/40496#issuecomment-705641649 for that.)

Reopening because the x/website CL was reverted in CL 261557. x/website needs to be modified when Go 1.16 is released. I'll move this to Go 1.17 milestone with early-in-cycle.

authors: ALTree dmitshur


text:
runtime/metrics: add contention metrics

Right now lock contention is only possible to debug with pprof using the block profile. This is very useful once contention has been identified as the issue, but since it has to be turned on manullay it doesn't help in identifying that contention is an issue. Exporting the cumulative wait time via runtime/metrics would allow continouous monitoring of contention and help in debugging Go programs.

cc @mknyszek @aclements

Interesting... Looks like we already measure time spent blocked for starvation purposes anyway. I suppose this wouldn't be too hard to expose. Is cumulative wait time the right metric? It's useful for comparison, but it doesn't really tell you much on its own.
An approximate distribution of latencies might be more useful for this, because you can correlate that with e.g. request latency, but it's slightly more expensive and it requires a bit more post-processing. Its other downside is that it's approximate -- I'm not sure if there's some special use case enabled by having a precise cumulative wait time.

To be clear I'm not opposed to cumulative wait time. If that's the convention we can move forward with that. Just want to make sure we considered our options.

Latency distribution would be even better. Approximate time would be good enough for all use cases I can think of.

cc @jeremyfaller

Moving to Backlog.  Please recategorize as appropriate.  Thanks.

Would it be a lot of work to get this into 1.20? It would help a lot in understanding performance issues in production jobs and a lot of Google internal users have expressed interest in this.

Given that there's known demand for this and that I think it wouldn't be hard to add (though I might be wrong), I'll move this to 1.20, but given our focus on core project health and PGO for 1.20, I'm not sure we'll get to this.

I'll probably do some other runtime/metrics stuff this release, so I'll put it on my plate.

authors: aclements ianlancetaylor mknyszek prattmic znkr