text:
Default to opaque white color in more CanvasItem::draw_* methods

Makes the color parameter default to opaque white (Color(1, 1, 1, 1)) for these methods:

CanvasItem::draw_dashed_line,
CanvasItem::draw_line,
CanvasItem::draw_polyline,
CanvasItem::draw_arc,
CanvasItem::draw_multiline,
CanvasItem::draw_rect,
CanvasItem::draw_circle,
CanvasItem::draw_colored_polygon,
RenderingServer::canvas_item_add_line,
RenderingServer::canvas_item_add_rect,
RenderingServer::canvas_item_add_circle.

Resolves https://github.com/godotengine/godot-proposals/issues/7310.

Notes:

In the code there were used Color(1, 1, 1, 1), Color(1, 1, 1), Color(1.0, 1.0, 1.0). They are equivalent, I've changed all of them to Color(1, 1, 1, 1) which seemed to be used the most. Could be changed to some other variant if preferred. Edit: it was done in canvas_item.h/cpp. But e.g. in RenderingServer.h/cpp Color(1, 1, 1) seems to be used instead, haven't touched these.

I haven't added defaults for the RenderingServer::canvas_item_add_* methods.

I think we should do that as well for consistency.

Looks like this will require compatibility methods for GDExtension and C#. Take a look at the "files changed" tab for more info

I haven't added defaults for the RenderingServer::canvas_item_add_* methods.

I think we should do that as well for consistency.

Changed some.

Looks like this will require compatibility methods for GDExtension and C#. Take a look at the "files changed" tab for more info

Added compat methods, based on #76577. Kinda no idea what I'm doing though. :upside_down_face:

We currently don't generate compat methods for C#, so they have to be added manually:
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Compat.cs b/modules/mono/glue/GodotSharp/GodotSharp/Compat.cs
index 41cea031ab..b8f8ff7927 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Compat.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Compat.cs
@@ -24,6 +24,65 @@ partial class AnimationNode
     }
 }

+partial class CanvasItem
+{
+    /// <inheritdoc cref="DrawArc(Vector2, float, float, float, int, Nullable{Color}, float, bool)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawArc(Vector2 center, float radius, float startAngle, float endAngle, int pointCount, Color color, float width, bool antialiased)
+    {
+        DrawArc(center, radius, startAngle, endAngle, pointCount, new Nullable<Color>(color), width, antialiased);
+    }
+
+    /// <inheritdoc cref="DrawCircle(Vector2, float, Nullable{Color})"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawCircle(Vector2 position, float radius, Color color)
+    {
+        DrawCircle(position, radius, new Nullable<Color>(color));
+    }
+
+    /// <inheritdoc cref="DrawColoredPolygon(Vector2[], Nullable{Color}, Vector2[], Texture2D)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawColoredPolygon(Vector2[] points, Color color, Vector2[] uvs, Texture2D texture)
+    {
+        DrawColoredPolygon(points, new Nullable<Color>(color), uvs, texture);
+    }
+
+    /// <inheritdoc cref="DrawDashedLine(Vector2, Vector2, Nullable{Color}, float, float, bool)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawDashedLine(Vector2 from, Vector2 to, Color color, float width, float dash, bool aligned)
+    {
+        DrawDashedLine(from, to, new Nullable<Color>(color), width, dash, aligned);
+    }
+
+    /// <inheritdoc cref="DrawLine(Vector2, Vector2, Nullable{Color}, float, bool)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawLine(Vector2 from, Vector2 to, Color color, float width, bool antialiased)
+    {
+        DrawLine(from, to, new Nullable<Color>(color), width, antialiased);
+    }
+
+    /// <inheritdoc cref="DrawMultiline(Vector2[], Nullable{Color}, float)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawMultiline(Vector2[] points, Color color, float width)
+    {
+        DrawMultiline(points, new Nullable<Color>(color), width);
+    }
+
+    /// <inheritdoc cref="DrawPolyline(Vector2[], Nullable{Color}, float, bool)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawPolyline(Vector2[] points, Color color, float width, bool antialiased)
+    {
+        DrawPolyline(points, new Nullable<Color>(color), width, antialiased);
+    }
+
+    /// <inheritdoc cref="DrawRect(Rect2, Nullable{Color}, bool, float)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public void DrawRect(Rect2 rect, Color color, bool filled, float width)
+    {
+        DrawRect(rect, new Nullable<Color>(color), filled, width);
+    }
+}
+
 partial class CodeEdit
 {
     /// <inheritdoc cref="AddCodeCompletionOption(CodeCompletionKind, string, string, Nullable{Color}, Resource, Nullable{Variant})"/>
@@ -81,6 +140,30 @@ partial class RenderingDevice
     }
 }

+partial class RenderingServer
+{
+    /// <inheritdoc cref="CanvasItemAddCircle(Rid, Vector2, float, Nullable{Color})"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void CanvasItemAddCircle(Rid item, Vector2 pos, float radius, Color color)
+    {
+        CanvasItemAddCircle(item, pos, radius, new Nullable<Color>(color));
+    }
+
+    /// <inheritdoc cref="CanvasItemAddLine(Rid, Vector2, Vector2, Nullable{Color}, float, bool)"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void CanvasItemAddLine(Rid item, Vector2 from, Vector2 to, Color color, float width, bool antialiased)
+    {
+        CanvasItemAddLine(item, from, to, new Nullable<Color>(color), width, antialiased);
+    }
+
+    /// <inheritdoc cref="CanvasItemAddRect(Rid, Rect2, Nullable{Color})"/>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void CanvasItemAddRect(Rid item, Rect2 rect, Color color)
+    {
+        CanvasItemAddRect(item, rect, new Nullable<Color>(color));
+    }
+}
+
 partial class RichTextLabel
 {
     /// <inheritdoc cref="PushList(int, ListType, bool, string)"/>

Added compat methods, based on #76577. Kinda no idea what I'm doing though.

Thanks! I've skimmed them and they look good to me. :-)
Unfortunately, they won't actually work until after PR https://github.com/godotengine/godot/pull/79702 is merged. I think it'd be good for that to be merged first, so that CI can double check that you've gotten them all correct.

Unfortunately, they won't actually work until after PR #79702 is merged. I think it'd be good for that to be merged first, so that CI can double check that you've gotten them all correct.

Rebased, seeing #79702 was merged. Hopefully they are correct, will see.
Anyway, thanks all of you for the help with this! :slightly_smiling_face:

Unfortunately, they won't actually work until after PR #79702 is merged. I think it'd be good for that to be merged first, so that CI can double check that you've gotten them all correct.

Rebased, seeing #79702 was merged. Hopefully they are correct, will see.

Unfortunately, CI is still not happy:
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_arc', from CFD4FBAB to 6031513E. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_colored_polygon', from 62E3D9E1 to B5F97BEA. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_dashed_line', from 81A7290C to EEC0FBE7. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_line', from 96057C42 to 89E57661. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_multiline', from FC2AB533 to 3BFB36E2. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_polyline', from F8E6DB22 to C0C2AA6D. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/CanvasItem/methods/draw_rect', from 0507B53D to B5FC4F88. This means that the function has changed and no compatibility function was provided.

Looking at the code, perhaps it's because all the orignal DEFVAL()'s aren't provided when registering the compatibility methods?
For example, for draw_arc() I think it maybe should be:
ClassDB::bind_compatibility_method(D_METHOD("draw_arc", "center", "radius", "start_angle", "end_angle", "point_count", "color", "width", "antialiased"), &CanvasItem::draw_arc_compat_79666, DEFVAL(-1.0), DEFVAL(false));

Looking at the code, perhaps it's because all the orignal DEFVAL()'s aren't provided when registering the compatibility methods?

I was looking at this for some time and everything looked the same to me. Now that you've pointed it out it's so obvious, definitely needed another pair of eyes. Thanks again!
I wonder why it wasn't complaining about RenderingServer::canvas_item_add_line though, I've missed default values in compat binding for it too. :thinking:

authors: Calinou clayjohn dsnopek kleonc raulsntos


text:
[Native File Dialogs] Refocus last focused window on close.

Fixes https://github.com/godotengine/godot/issues/80905

Thanks!

source: gharchive/pull-request

authors: akien-mga bruvzg

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Add icons to some editor classes

A few concerns: What about nodes vs refcounteds and objects? We don't really use color for anything other than nodes and resources as of now, with the only exception being Tween. But maybe editor-only objects I'll still give them a tint if we add icons to any in the future.

Oops botched it up. EditorCommandPalette is meant to be this:

Why was this blocked again? I remember some discussion about the use of colored icons vs drab icons but nothing recent.

Looks good to me, but I can't figure out how to display editor-only nodes in the Create New Node dialog so I can preview the icons (including in light mode).

Light mode was taken into account, but if you want to check it out, you should pull up F1.

Rebased and tweaked the inspector icon into this simpler one:

I don't know what could be causing checks to fail.

You probably need to rebase. There was some CI issue.

authors: Calinou KoBeWi MewPurPur fire


text:
Make sure Window's title is respected before we compute the size

Fixes https://github.com/godotengine/godot/issues/85118.
Like the title says, we used to first apply the size limit, and then we would update said limit without it affecting the actual size. So the window would be displayed with one size, and window's contents would refer to another size. Good old DisplayServer/Window de-sync.
It may not be Windows specific, as far as OSes go, highly depends on how window management is implemented on each platform. But since we are only reproducing it on Windows for now, I'm marking it as such. Also I guess we can say it's a regression because it's been introduced with the feature to keep the title visible in 4.2: https://github.com/godotengine/godot/pull/80409.

Also removes some suspicious and outdated code that forced this particular dialog to change size when the warning message changed.

Thanks!

authors: YuriSizov akien-mga


text:
Instruct users to add code/errors as text in issues

Input on the wording and specifics appreciated

Thanks!

Thank you!

authors: AThousandShips akien-mga


text:
Update the description of the method get_connection_list in GraphEdit

The structure of the connection (of type dictionary) in returned array is actually { from_port: 0, from_node: "GraphNode name 0", to_port: 1, to_node: "GraphNode name 1" }

Bugsquad edit, fixes: https://github.com/godotengine/godot-docs/issues/8625

Thanks! And congratulations on your first merged Godot contribution!

Thanks! And congratulations on your first merged Godot contribution!

Oh，I would say THANK YOU! You encouraged me to do this when I was (am still) just a rookie to github and godot.

Cherry-picked for 4.2.2.

authors: YuriSizov ZeferinoI


text:
Refactor templates and code duplication

So I was searching for bind_static_vararg_method (which sadly doesn't exist), and ended up reading Godot's template code.  I noticed that there was a lot of duplication and decided to see if I could reduce the duplication.
I've summarized the changes and how I tested it below since template code is often difficult to read without knowing the context.

Collapse multiple, duplicate, specializations into single specializations (with the help of enable-if).
For example, thistemplate<typename T> struct MyStruct;
template<> struct MyStruct<MyType> { /* some specialization */ };
template<> struct MyStruct<const MyType&> { /* identical specialization as just "MyType"*/ };

Becomestemplate<typename T, typename = void> struct MyStruct;
template<typename T> struct MyStruct<T, EnableIf_t<types_are_same_v<MyType, RemoveRefPointerConst_t<T>>> {
  /* some specialization */
};

Move existing templates to typedefs.h since one file couldn't include core/typedefs.  Figure it doesn't hurt to move them since it increases visibility
Add get_type_name() to GetTypeInfo due to the refactor in typed_array.h.  I think the correct place to put this string is in PropertyInfo, but wanted to see what others thought first.
I'm assuming GetTypeInfo<T>::get_class_info().class_name is T::get_class_static() for Objects and empty string otherwise.
VariantObjectClassChecker was not collapsed, but instead was generalized to handle const and non-const Ref.  I figured this was necessary since the place its used (VariantCasterAndValidate), can pass in any type.
Add a few new template types to core/typedefs.h that are similar, but not the same, as stl templates:

is_void_v returns void if the base type is void, instead of exactly void
Test:
print_line("std::is_void_v",
           std::is_void_v<void>,
           std::is_void_v<void*>,
           std::is_void_v<void*&>,
           std::is_void_v<void**>,
           std::is_void_v<const void*>,
           std::is_void_v<const void*&>,
           std::is_void_v<const void**>,
           std::is_void_v<const void**&>);
print_line("is_void_v",
           is_void_v<void>,
           is_void_v<void*>,
           is_void_v<void*&>,
           is_void_v<void**>,
           is_void_v<const void*>,
           is_void_v<const void*&>,
           is_void_v<const void**>,
           is_void_v<const void**&>);

Output:
std::is_void_v true false false false false false false false
is_void_v true true true true true true true true

RemoveRefPointerConst_t removes const, ref, and pointer(s) from a type, leaving you with the base type.
I tried std::decay too, but it returned false for every case.
Test:
print_line("std",
           std::is_same_v<int, std::remove_cv_t<std::remove_reference_t<int>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<int&>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<int*>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<int*&>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<int**>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<int**&>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<const int&>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<const int*>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<const int*&>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<const int**>>>>,
           std::is_same_v<int, std::remove_cv_t<std::remove_pointer_t<std::remove_reference_t<const int**&>>>>);

print_line("custom",
           std::is_same_v<int, RemoveRefPointerConst_t<int>>,
           std::is_same_v<int, RemoveRefPointerConst_t<int&>>,
           std::is_same_v<int, RemoveRefPointerConst_t<int*>>,
           std::is_same_v<int, RemoveRefPointerConst_t<int*&>>,
           std::is_same_v<int, RemoveRefPointerConst_t<int**>>,
           std::is_same_v<int, RemoveRefPointerConst_t<int**&>>,
           std::is_same_v<int, RemoveRefPointerConst_t<const int&>>,
           std::is_same_v<int, RemoveRefPointerConst_t<const int*>>,
           std::is_same_v<int, RemoveRefPointerConst_t<const int*&>>,
           std::is_same_v<int, RemoveRefPointerConst_t<const int**>>,
           std::is_same_v<int, RemoveRefPointerConst_t<const int**&>>);

Output:
std true true true true false false true true true false false
custom true true true true true true true true true true true

static_or_reinterpret_cast (for lack of a better name, considering it optionally calls const_cast too) to simplify otherwise complex expressions.
For example, this
T *ret1 = const_cast<T *>(*reinterpret_cast<T *const *>(p_ptr));
RefCounted *ret2 = const_cast<RefCounted *>(static_cast<const RefCounted *>(o));

Becomes
T *ret1 = static_or_reinterpret_cast<T*>(p_ptr);
RefCounted *ret2 = static_or_reinterpret_cast<RefCounted *>(o);

Test code to verify that the correct casts were taking place (I also put printfs in static_or_reinterpret_cast so I can see which one is being called)
Label* l = memnew(Label);
Node* n0 = static_or_reinterpret_cast<Node*>(l);
Node* n1 = static_or_reinterpret_cast<Node*>((const Label*)l);
Node* n2 = static_or_reinterpret_cast<Node*>((void*)l);
Node* n3 = static_or_reinterpret_cast<Node*>((const void*)l);
const Node* n4 = static_or_reinterpret_cast<const Node*>(l);
const Node* n5 = static_or_reinterpret_cast<const Node*>((void*)l);
const Node* n6 = static_or_reinterpret_cast<const Node*>((const Label*)l);
const Node* n7 = static_or_reinterpret_cast<const Node*>((const void*)l);

The original reason for this function is that collapsing some duplicate template specializations didn't result in identical code.  In particular, const_cast was always called instead of conditionally called.  struct PtrToArg<T, EnableIf_t<std::is_pointer_v<T>>> in method_ptrcall.h is a good example of this.

is_ptr_to_const_v determines if a pointer is a pointer-to-const (like const int*).
This template was needed for static_or_reinterpret_cast since using const_cast is dependent on whether it is working on a pointer-to-const or not.  This does not return true for ref-to-const.
Test:
 print_line("is_ptr_to_const_v",
            is_ptr_to_const_v<int>,
            is_ptr_to_const_v<int&>,
            is_ptr_to_const_v<int*>,
            is_ptr_to_const_v<int*&>,
            is_ptr_to_const_v<int**>,
            is_ptr_to_const_v<int**&>,
            is_ptr_to_const_v<const int&>,
            is_ptr_to_const_v<const int*>,
            is_ptr_to_const_v<const int*&>,
            is_ptr_to_const_v<const int**>,
            is_ptr_to_const_v<const int**&>);

Output:
is_ptr_to_const_v false false false false false false false true true true true

Marking myself as off-topic but holy §&%$.

While the general style and usage needs to be evaluated it does appear to provide a very slight but real improvement to build time on Windows, unsure what exactly causes it though
These changes are relatively general and in various areas, so might be easier to review this by splitting it into a few separate PRs with specific focus

I would say however that I'm not so keen on the static_or_reinterpret_cast, it obfuscates what's going on and I'd prefer explicit casting to not cause any confusion

If I remove it, there will be some templates that will always const_cast even when it's not needed.  I don't think there's any harm, and, it's possible the compiler notices this and optimizes it away.  So it's probably okay to leave the change out. I won't know for sure until I try though.
As for splitting the PR, it would turn into these PRs (optimistically)

Delete unnecessary forwards to TypedArray (trivial, assuming it still compiles)
Add get_type_name() to GetPropertyInfo (trivial, though on the surface it adds nothing to pre-existing code)
Collapse templates with EnableIf_t (still a big change, but more manageable)

This one might be able to be further split by which template is being simplified.  The main ones are PtrToArg, TypedArray, GetTypeInfo, and a handful that only exist in binder_common.h.

At a high-level, simplifying these templates would be very welcome, since they are quite tricky to understand and maintain

Thanks!

This touches a very large amount of the code responsible for encoding/converting arguments for making ptrcalls to GDExtension, and we don't want to accidentally break binary compatibility.

Agree.  I don't have a way to test GDExtension (thoroughly) unfortunately.

I've managed to split this PR into five smaller PRs.
While splitting this PR...

I did not add get_type_name() to GetPropertyInfo since it seemed kind of hacky.  Instead I added a new template called (for a lack of a better name) GetNameHelper, which returns either T::get_static_class() or Variant::get_type_name(GetTypeInfo<T>::VARIANT_TYPE) depending on type T.  It is likely that this could be moved somewhere else (maybe variant.h), but I'll wait for what reviewers say when we get to that PR.
At @AThousandShips 's request, I deleted static_or_reinterpret_cast.  Please note that this means we'll use const_cast even when it's unnecessary in some templates.
is_ptr_to_const_v and is_void_v were deleted because static_or_reinterpret_cast was deleted.
any additional deltas made by static_or_reinterpret_cast were reverted

The squashed result of the five PRs does not equal this PR due to the changes mentioned above, but it's pretty close.
I'll ask for reviews for one or two PRs at a time since the latter PRs depend on the former.

authors: AThousandShips Mickeon dhoverml


text:
Remove temporary project conversion

This project conversion affects only projects, that were edited after #97707 (v4.4-dev4) and provides an easy way to upgrade these projects.
It seems reasonable to assume, that at that time of the next stable Godot-release all affected projects have been converted.
So I would like to suggest to merge this PR and remove the project conversion shortly before the next stable release,
For projects, that haven't been converted, we can provide an upgrade-strategy, that that consists of opening the project once with v4.4-dev5.
Follow-up to/Implements https://github.com/godotengine/godot/pull/99449#pullrequestreview-2449032498

I have updated the PR to additionally revert #100334.


text:
Add build support for Android

Current build guide at README.md also works with this PR to build for Android.
During development the following pre-compiled binaries were used for Android.
To build for Android, use platform=android.
ANDROID_NDK_ROOT env variable needs to be defined.
By default, it builds for armv7 with Api 18.
To build for arm64v8 (or x86, x86_64), use android_arch=arm64v8.
64 bits requires at least Api level 21.
If needed, you can change it with android_api_level=XX.

This looks really interesting, I'll try to review it this week. :+1:

Just wanted to add +1 that I used this last week and outputted working android binaries for the plugin. Thanks!

Thanks!

authors: Faless lewispollard rondao

repo: godotengine/webrtc-native


text:
[TESTING] Testing for unlock legacy withdrawal cells feature

This is a temporary PR for testing only, close it after the testing completed.
The feature can only be tested on mainnet_v0, so please use the Vercel mainnet preview link to test it.
The PR is based on feat-v0-unlock branch, changes:

Remove owner_lock from withdrawal_request v0, so after the withdrawal cell is finalized, user need to manually unlock it on godwoken-bridge

test completed.

repo: godwokenrises/light-godwoken


text:
建议gcron增加StartAll

框架启动，通过gcron.Add添加多个定时任务后还要一个个start，建议增加一个StartAll方法，一次性启动所有的定时任务。

@jialeo 哥，gcron.Add添加的任务自动就会启动啊，不需要再调用Start方法。

source: gharchive/issue

authors: gqcn jialeo

repo: gogf/gf


text:
os/grpool: Add WithParams Feature

os/grpool: Add WithParams Feature

@CyJaySong You can pass parameters using ctx.

@CyJaySong You can pass parameters using ctx.
需要传递多个参数时，需多次使用context.WithValue()代码看起来很不美观

authors: CyJaySong gqcn


text:
Avatar-related settings feedback

This checkbox should probably be hidden if gravatar is disabled.

Enable Custom Avatar   Enable this to disable fetch from Gravatar

Add a button to delete current uploaded avatar
Display current custom avatar

Thanks your feedback!

It should be noted that the user profile also still has the gravar tooltip and hyperlink when gravatars are disabled.

Be sure you read the tooltip by each world.

Adjust some changes about avatar, and you can delete current avatar now.

authors: Unknwon xPaw xor-gate

repo: gogits/gogs


text:
Treat docker images as a whole, not as individual files

I have an example where a docker image was published and one of the base layers of the image doesn't change.  So as newer CI versions of the image are published, that layer file is always the same.  Now when I want to purge based on age, lavatory is identifying this base layer image file to be deleted even though it is required of the newest full docker image.
Maybe I'm not understanding a needed aspect of how to do an artifactory query or something for this use case?  Basically I would consider the age of the image and later files to all be the age of the manifest.  I would expect artifactory itself handles the reference counting on this common layer file used in multiple versions of the image.

What is the structure you have for your images?
We basically have a structure as
path/image1/version1
path/image1/version2
path/image2/version1
...

We have a query, similar to the first complicated example using manifests.json. Then we find the parent directory and delete that one.
Its essentially something like
def purgelist(artifactory):

    docker_terms = [...]

    matches = artifactory.filter(terms=docker_terms, depth=None, item_type='file')
    purgeable = []
    for entry in matches:
        root_path, _ , directory_name = entry['path'].rpartition('/')
        entry.update({
            'path': root_path,
            'name': directory_name,
            'type': 'folder',
        })
        purgeable.append(entry)

    return purgeable

Structure could vary as we allow teams to name images how they want.
I guess I see now that I would need to tailor this function to be the selector of the image level folders.  I guess I wasn't seeing a specific example to implement somthing like this in the docs.
I wrote a bash script using the artifactory API that I think does basically what you are suggesting.  I look for the manifest.json files that are outside of the retention rules that I want and then delete from the folder level that contains the manifest.json which will be the whole image.  I guess I didn't understand the framework here enough (and I'm not much of a python guy) to implement clearly within the lavatory rule

Understood.
The trick is basically getting the parent directory that has the manifest.json that should be purged.
Any feedback / PRs on improving docs would be beneficial for other folks. :+1:

authors: NetAppBlueDevil sijis

repo: gogoair/lavatory

license_source: bigquery


text:
[BUG] Bootstrap install script leaves localhost out of the interface list, but it is currently required for cert connections

Please describe the issue.
Bootstrap install script leaves localhost out of the interface list, but it is currently required for cert connections.
When inputting values for the FarmerInterface, localhost isn't added automatically, but it needs to be until the farmer can automatically generate a cert for it.
Version Information
Latest
Does this bug exist on the tip of master?
[X] Yes
[ ] No

closed by https://github.com/gogrlx/bootstrap.grlx.dev/commit/a773d800697c02bd16c6b14fc84eb35f28b54ffb

repo: gogrlx/grlx


text:
.Content doesnt rebuild when dynamically-called partial inside hooks modified

What version of Hugo are you using (hugo version)?

hugo v0.106.0-DEV-77fc74a5b20f50298ac4a1cd88e436932fc2226f+extended linux/amd64 BuildDate=2022-10-28T12:41:15Z VendorInfo=mage

Does this issue reproduce with the latest release?
Yes
Bug
In server mode .Content doesnt rebuild when dynamically-called partial template inside markdown render hooks modified
How-to reproduce

start hugo server, then open browser.
modify layouts/partials/dynamic-render-link.html,
Add some random character
Changes detected, site rebuild but .Content doesnt update with new changes

All markdown hooks affected, link,image, heading, codeblock.
Also tested using --disableFastRender still doesnt work.
Demo
https://github.com/pamubay/markdown-hooks-dynamic-call
Thank you.

Similar to, or perhaps a duplicate of, https://github.com/gohugoio/hugo/issues/8863.

I'm closing this as a duplicate. I'm currently working on a more robust setup that should handle this.

authors: bep jmooring pamubay

repo: gohugoio/hugo


text:
Issue with md file name contains a ".",

My English is poor, please use the Google translator
在0.38版中...当md文件名中带有.时,生成的html会忽略.以后的字符串,最终生成的html文件名会被截取...
而0.37.1不会出现这个问题

In the 0.38 version... When the md file name contains a ".", the generated html will ignore the subsequent strings, and the resulting html file name will be intercepted...
0.37.1 does not have this problem

I am experiencing the same issue starting from Hugo version 0.38.
The stats all show 559 pages for one of my websites but with 0.38 and 0.38.1 some pages are missing:
With Hugo 0.38 and 0.38.1:
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com.Tecnick
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com.Tecnick.Color
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model
tcpdf.org/public/docs/srcdoc/tc-lib-color/index.html

With Hugo 0.37.1:
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Css
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Exception
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model.Cmyk
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model.Gray
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model.Hsl
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model.Rgb
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Model.Template
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Pdf
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Spot
tcpdf.org/public/docs/srcdoc/tc-lib-color/class-Com.Tecnick.Color.Web
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com.Tecnick
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com.Tecnick.Color
tcpdf.org/public/docs/srcdoc/tc-lib-color/namespace-Com.Tecnick.Color.Model
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Css
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Exception
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model.Cmyk
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model.Gray
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model.Hsl
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model.Rgb
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Model.Template
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Pdf
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Spot
tcpdf.org/public/docs/srcdoc/tc-lib-color/source-class-Com.Tecnick.Color.Web
tcpdf.org/public/docs/srcdoc/tc-lib-color/index.html


text:
basic example doesn't return data

just following the basic example in jupyter.
Current Behavior
online_features = client.get_online_features(
feature_refs=[
f"daily_transactions",
f"total_transactions",
],
entity_rows=[
GetOnlineFeaturesRequest.EntityRow(
fields={
"customer_id": Value(
int64_val=1001)
})
],

)
print(online_features)
produces:
field_values {
fields {
key: "customer_id"
value {
int64_val: 1001
}
}
fields {
key: "customer_project2/daily_transactions"
value {
}
}
fields {
key: "customer_project2/total_transactions"
value {
}
}
}
note that there does appear to be data in the kafka features topic.  I'm not sure if that is where the online feature service is getting the data or whether it is in redis.

Hello @BenMacKenzie
Last time some other people find the same issue, but I cannot replicate it yet.
May I know your Platform specification?
OS
Docker Version

Thanks

note that there does appear to be data in the Kafka features topic. I'm not sure if that is where the online feature service is getting the data or whether it is in Redis.

Normally the data will be pushed to Kafka, and the Ingestion Job is going to Ingest the Data from Kafka to Redis

@BenMacKenzie Have you made any changes to the max_age?

I just re-did everything and it works fine.  Maybe while struggling with the pandas issue more than 432000 seconds elapsed (5 days)?
thanks!

authors: BenMacKenzie imjuanleonard woop

repo: gojek/feast


text:
Allow use of secure gRPC in Feast Python client.

What this PR does / why we need it:
Since #458 enables provisioning of an nginx ingress with TLS, when connecting from outside the cluster, secure gRPC is required.
Which issue(s) this PR fixes:

N/A
Does this PR introduce a user-facing change?:

If Feast Core or Serving gRPC APIs are listening on port 443, the Feast Client will default to enabling client-side TLS. If using non-secure gRPC on port 443, explicitly set the secure flag to false when constructing the client.

/assign @davidheryanto

Thanks for this @Yanson! Will try to have this reviewed soon.

/ok-to-test

@Yanson this PR looks good, but I think you will need to implement tests so that we know we aren't regressing in the future. Currently this is untested.

/retest

/approve

/lgtm

authors: Yanson woop zhilingc


text:
Constant backoff actually has 0 backoff on first retry

When attempting to use constant backoff strategy, I found that on the first retry of a request, the actual backoff time is 0. That's because on the first retry the client calls c.retrier.NexInterval(0) so constantBackoff.Next(0) returns 0:
https://github.com/gojek/heimdall/blob/16e67fd885aa9cb5b9d2257936e8e8c301eba52a/backoff.go#L34

The same thing happens when using exponentialBackoff:
https://github.com/gojek/heimdall/blob/16e67fd885aa9cb5b9d2257936e8e8c301eba52a/backoff.go#L60

@btamadio Thanks for pointing this out. Can you raise a PR for the same, will have a look

Here's an example demonstrating the bug:
func main() {
	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
		w.WriteHeader(500)
	}))
	defer s.Close()
	cli := httpclient.NewClient(
		httpclient.WithRetryCount(3),
		httpclient.WithHTTPClient(s.Client()),
		httpclient.WithRetrier(
			heimdall.NewRetrier(
				heimdall.NewConstantBackoff(
					2*time.Second,
					2*time.Millisecond,
				),
			),
		),
	)
	cli.AddPlugin(plugins.NewRequestLogger(os.Stdout, os.Stderr))
	_, _ := cli.Get(s.URL, nil)
}

The first retry happens with zero backoff. The backoff is only applied starting from the second retry, as you can see from the timestamps:
21/May/2020 10:53:39 GET http://127.0.0.1:51137 500 [0ms]
21/May/2020 10:53:39 GET http://127.0.0.1:51137 500 [0ms]
21/May/2020 10:53:41 GET http://127.0.0.1:51137 500 [1ms]
21/May/2020 10:53:43 GET http://127.0.0.1:51137 500 [0ms]

authors: btamadio rShetty

repo: gojek/heimdall


text:
Improve version list and details page:

What this PR does / why we need it:

Action buttons in version list now displayed using column direction
Fix endpoint url for pending version deployment in version details page

Which issue(s) this PR fixes:

Fixes #36
Checklist

[x] Tested locally

Any screenshots?

Any screenshots?

authors: ariefrahmansyah pradithya

repo: gojek/merlin


text:
SVG App

Am calling this an App since the intent is a Inkscape like experience.
What follows is a complete spec ( if you will ) to get it all in one place for now.
Spec:
One of my long-term life goals is have a decent replacement for InkScape.
Mac, Windows and Linux.
Current status:

All the svg rendering is in https://github.com/goki/gi/tree/master/svg
Sprites and a transparent overlay available for rendering edit controls (control points, rubber bands, etc) on top of the underlying drawing, without affecting anything else.
The cursor in TextField is a sprite, and drag-n-drop uses the overlay, so you can look at those for examples..

High Level approach:

Get basic Window and Canvas working such that screen DPI and horizontal / vertical scrolling work ok on all OS's first.

User editing via Property Editor: Get the 2D SVG rendering working, where the user can edit the "widgets" in the IDE by adding and removing nodes and changing the properties of elements.

User editing via Canvas. Get an event loop going so that users can use mouse / touch to move, scale, rotate elements, such that the properties in the bullet point above
are changed.

Have it savable and loadable from disk.

Have it parametric, such that changing a property results in lots of other elements adjusting as well. A simple example is Group scaling where you select many objects and they scale together.
The IDE / Canvas can be used to create the links between objects to be parametric.
Standard Transformation like scale, rotate, initially.
Custom Transformations used form the Data Flow system.
The parametric relationship should be described in the File format so that its not lost.

Make a DataFlow system. Have the Parametric function customisable using the SVG system to build a Data FLow system. This allows users to setup complex transformation flows and run them in the GUI. Its almost a separate project.

Everything except the last point is now in place (more-or-less) in the https://github.com/goki/grid app..
If this is still relevant, it would be more appropriate as a discussions topic:

Make a DataFlow system. Have the Parametric function customisable using the SVG system to build a Data FLow system. This allows users to setup complex transformation flows and run them in the GUI. Its almost a separate project.

authors: ghost rcoreilly

repo: goki/gi


text:
Padding area must always have the background color of the element

Currently, the padding area is transparent/black on some elements like layouts.

This is due to the fact that layouts do not render at all. This is not clearly documented on the layout type as it should be here.


text:
Added testFrameInit()

Checklist

[ ] New Extension
[x] New Test
[ ] Changed more than one extension, but all changes are related
[ ] Trivial change (doesn't require changelog)

          2 Messages

  :book:
  Executed 134 tests, with 0 failures (0 unexpected) in 3.780 (3.992) seconds

  :book:
  Executed 125 tests, with 0 failures (0 unexpected) in 2.968 (3.389) seconds

  Generated by :no_entry_sign: danger

What are you thinking about the selector test?

authors: EZSwiftExtensionsBot goktugyil lfarah

repo: goktugyil/EZSwiftExtensions

license: mit


text:
Fix Callout size(In Hight)

Appearance looking like it right now

Make It look bit minimalist in appearance

hello, thanks for raising this issue
does this look ok?

fixed in v2
you can now update the theme

Great work

license: Unlicense


text:
[WIP] Prototype doc site

What does this do / why do we need it?
This is a prototype for a dep doc site. You can see an example of it at https://carolynvs.github.io/depdocs/.

It uses Hugo as the static site generator.
The docs source continue to be located in /docs, and are versioned along with the dep code.
The site is hosted on github pages and the generated files are in the gh-pages branch. This branch should never be manually tweaked as changes will be lost on the next push.
The site would be located at https://golang.github.io/dep/.
Travis is configured to generate the site for pushes to master, and for tagged releases.
Tagged releases are published to /releases/vX.Y.Z/. The sidebar links to these manually (but an enterprising person would be welcome to automate this part).
We will not go back and update older releases doc. Whatever was present when we tagged, is what is included.
There is an encrypted deploy key checked into hack which travis uses to push to gh-pages. PR builds do not have access to this, it's only decrypted for merged code.
The modified/forked theme is located at https://github.com/carolynvs/hugo-alabaster-theme/tree/depdocs. I am exporting it and checking it into our repo so that we don't mess with submodules or anything fussy.

Before we merge this the following should occur. I'll do this on this PR after we are sure we want to accept this PR:

[ ] Remove the old standalone doc files and move the content into the generated site.
[ ] Update all links
[ ] Remove duplicate content from the readme and link to the site. We'll keep dev specific sections like contributing/feedback, but usage, etc should not be duplicated.
[ ] Document how to preview the site locally (here's the makefile I am using in the example site)
[ ] Add the deploy key to this repo.

What should your reviewer look out for in this PR?

Is this a workable initial setup for our doc site?

What is not in scope?

It doesn't have to be a drop in / migration ready setup for when dep is merged into the toolchain.
I don't have any frontend aspirations and would like to avoid any changes to the theme or visuals. Hopefully we can find someone with more design skills to tackle and incrementally improve the site.
I'm explicitly not adding new content or majorly reorganizing it. Once we have the infrastructure in place, we can tweak from there.

Do you need help or clarification on anything?
I don't believe so.
Which issue(s) does this PR fix?
Fixes #331. Or at least it's a start. 😀

ok so, we definitely want to do this 😄
i'll try for more review today or tomorrow, but in the meantime, do you think we could reasonably do some of those checkboxes in stages?

@sdboyer Yeah, all the checkboxes can be done later, though if we want deploys to work, adding the deploy key to the repo is not optional. Everything else can be done later and I think we should.

let's just get the deploy key in here, then. I'm not clear on what we need to do there, though - just enter it into Travis via their mechanism for secrets?

@sdboyer On the repo, click Settings -> Deploy Keys -> Add Deploy Key, then paste the public key from my pull request (below).  The private key has been encrypted with the travis tool for this repo only, and can only be decrypted during a travis build on master (i.e. not forks).
https://github.com/golang/dep/pull/1369/files#diff-4792ccfc9d3ef656e9cb864451527ea5

authors: carolynvs sdboyer

repo: golang/dep


text:
Do not acquire lock for file.Sync() fsync call

In the Flush() function, a lock is acquired before calling file.flush and file.sync. file.flush writes the glog buffer into the kernel buffer, while file.sync writes the kernel buffer to disk.
We encountered a problem where when disk I/O is heavy(the cloud service provider offers poor cloud disk performance), all glog logs would be blocked due to waiting for file.sync.
file.sync ensures that logs are flushed to disk, but I believe that acquiring a lock before calling file.sync is unnecessary. The kernel has an efficient buffered I/O mechanism, and file.sync does not block kernel buffered I/O. We only need to request that logs are flushed to disk and do not need to acquire a lock and wait for the kernel to complete the flush. Acquiring a lock and waiting for logs to be flushed to disk cannot make good use of the kernel's buffered I/O, and it can lead to performance issues.

Hi @chressie @stapelberg  , Could you please take a look at this pull request when you have a chance? I'd appreciate your review and feedback.

Hi @1978629634.
Thanks for your PR and for taking the time to explain and draft a fix. While the change you're requesting is reasonable, we cannot merge it directly into this repository. We have to make an internal change first, that we'll then be able to push upstream.
Quote from the README:

The repository contains an open source version of the log package used inside Google. The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored.
Send bug reports to golang-nuts@googlegroups.com.

I'm closing this PR and will soon merge a new version with a fix. Thanks for your understanding.

repo: golang/glog


text:
x/mobile: update golang/mobile docker container

Current repo requires the latest Go.
https://go-review.googlesource.com/9077 updated the Dockerfile, but I don't know how to upload the newly built docker container.
cc: @rakyll

#10523 will deprecate docker-based build path, but still it is better to update.

repo: golang/go


text:
cmd/compile: type checking fails in cycle involving recursive struct and unsafe.Sizeof

package p
import "unsafe"

type hdr struct {
    next *msg
}

type msg struct {
    hdr
    pad [1024-hdrsize]uint8
}

const hdrsize = unsafe.Sizeof(hdr{})

This should type check but does not:
$ go tool compile /tmp/x.go
/tmp/x.go:14: invalid type for composite literal: hdr
/tmp/x.go:14: invalid expression unsafe.Sizeof(hdr literal)

Looks like the compiler gets confused by the not-quite-cycle in the definition.
http://play.golang.org/p/Hbmv1j_UrR
/cc @RLH

Could this also be the case (when using cgo):
phySize := unsafe.Sizeof(C.struct_ad9361_rf_phy)
giving a compile time error:
invalid expression unsafe.Sizeof(_Ctype_struct_ad9361_rf_phy)

@davmaz No, the problem there is simply that unsafe.Sizeof takes a value, not a type.  It's not the same as the C sizeof operation.
In general, for the Go project, please ask questions in a forum rather than on the issue tracker.  See https://golang.org/wiki/Questions.  Thanks.

The problem is that when typechecking a pointer to a named type like *msg, cmd/compile recurses and tries to completely typecheck msg.  Instead, we should just do the minimum to get a *Type value for msg (in this case a TFORW type) so we can construct the *msg pointer type.  Later when we visit msg's type declaration, we can finish typechecking it.
A workaround is to move the hdr type declaration after the msg type declaration.

Quite possibly Robert's front end revamp will take care of this. Maybe not worth worrying about before then.

FWIW, gotype accepts this code w/o problems. Since there's a work-around per @mdempsky I don't think there's any urgency here.

Reproduced at tip (00263a896856c12cb57ee17355c6f911252b6214) on May 10, 2017.
Still seems low priority; punting to 1.10.

FWIW, gotype accepts this code w/o problems. Since there's a work-around per @mdempsky I don't think there's any urgency here.

FYI, both go/types and cmd/compile now reports an error as of go1.21 and tip:
$ go tool compile p.go
p.go:5:6: invalid recursive type hdr
	p.go:5:6: hdr refers to
	p.go:9:6: msg refers to
	p.go:14:7: hdrsize refers to
	p.go:5:6: hdr
$ gotype p.go
p.go:5:6: invalid recursive type hdr
p.go:5:6: 	hdr refers to
p.go:9:6: 	msg refers to
p.go:14:7: 	hdrsize refers to
p.go:5:6: 	hdr

authors: cuonglm davmaz griesemer ianlancetaylor josharian mdempsky rsc


text:
encoding/json: unmarshalling same field into two struct fields

Please answer these questions before submitting your issue. Thanks!

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

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

What did you do?
I'm trying to unmarshal the same JSON field into two struct fields.
http://play.golang.org/p/K86eAWaVyr

What did you expect to see?
Struct fields must have JSON value or json.Unmarshal() method must return an error.

What did you see instead?
Struct fields are empty and no errors returned.

Hopefully this is fixable without compatibility worries.

I took a look at this. I don't have a complete answer for it, but want to document my findings here.
This behavior is caused by the "annihilation" logic shared between encoding and decoding. For encoding, this makes sense: since the two sources might have different value, we might as well not write anything. One might argue that this should be an error. I'm not sure what the rationale is, but the doc on dominantField function ( https://golang.org/src/encoding/json/encode.go#L1139 ) and a test (TestDuplicatedFieldDisappears) indicate that it's a deliberate decision.
Since encode makes a decision to not error, I think we should refrain from returning an error from the decode side. One way to solve this issue is to give decode its own version of dominantField that does "union" instead of "annihilation", setting all "conflicted" fields to the same value.
Of course, another way out is to document this behavior and keep the implementation as is. What do you think?
(Please let me know if golang-dev is a better place for this discussion.)

ping @bradfitz . Is this too late for 1.7 freeze?

Things aren't frozen yet, but this would need to have a decision and an implementation soon. I'm not familiar with any past decisions here.
/cc @rsc

I thought a little more about this. Currently dominantField chooses tag over field name. If we make a change along the way I earlier described ("union" instead of "annihilation") we now have to answer more questions like: If we have tagged fields, do we set the fields that are untagged anyway?
Furthermore, we will probably want to document this behavior to prevent confusion in the future. It would be confusing if Marshal and Unmarshal follow different rules. Rule for Marshal is already documented, so we might as well follow it.
I am leaning towards retaining the old behavior. Will this need to wait for 1.8? I'd appreciate your thoughts. @bradfitz @rsc

The annihilation logic is intentional. I don't think we can change this. This use case is already defined to ignore both. I apologize.

authors: bradfitz pongad rsc wlredeye


text:
x/tools/go/gcexportdata: fails on Windows

The dashboard is red for x/tools on Windows: see https://build.golang.org/ ....
https://build.golang.org/log/9b26b9bbccfc5cd7cf490b38423d8d8e196cffd2
--- FAIL: ExampleRead (0.00s)
got:
Package path:       fmt
Export data:        fmt.a
Package members:    [Errorf Formatter Fprint Fprintf Fprintln]...
Println type:       func(a ...interface{}) (n int, err error)
Println location:   $GOROOT\src\fmt\print.go:123:1
want:
Package path:       fmt
Export data:        fmt.a
Package members:    [Errorf Formatter Fprint Fprintf Fprintln]...
Println type:       func(a ...interface{}) (n int, err error)
Println location:   $GOROOT/src/fmt/print.go:123:1
--- FAIL: ExampleNewImporter (0.02s)
got:
const math.Pi untyped float = 3.14159 // $GOROOT\src\math\const.go:11:1
const TwoPi untyped float = 6.28319 // twopi.go:5:7
want:
const math.Pi untyped float = 3.14159 // $GOROOT/src/math/const.go:11:1
const TwoPi untyped float = 6.28319 // twopi.go:5:7
FAIL
FAIL	golang.org/x/tools/go/gcexportdata	0.067s

This is new code by @adonovan.

Sorry, I thought I'd fixed it with https://go-review.googlesource.com/c/31813/ but there's a second instance.  Fix pending...

Looks fixed.

authors: alandonovan bradfitz griesemer


text:
runtime: some Go executables broken with macOS 10.12.4 / Xcode 8.3 but fine with Xcode 8.2

See https://github.com/couchbase/sync_gateway/issues/2417 for repro steps.
System:
MacBook Pro (x86_64)
macOS 10.12.3 (Sierra)
go version go1.8 darwin/amd64
Xcode 8.3;  clang -v
Apple LLVM version 8.1.0 (clang-802.0.38)
Target: x86_64-apple-darwin16.4.0
Thread model: posix
What did you do?
See description at
https://github.com/couchbase/sync_gateway/issues/2417
I also found this with some code I am unable to share.
My code does work fine with Xcode 8.2:
clang -v
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin16.4.0
Thread model: posix
and from reading the bug linked above it works fine there as well.

I have the same problem as OP but only for some binaries, not all. When trying to start, it is killed instantly.
$ ./test
Killed: 9

I can't figure out the common pattern for broken programs.

Reproduction:
package main

import (
	"fmt"

	_ "github.com/veandco/go-sdl2/sdl"
)

func init() {
	fmt.Println("init")
}

func main() {
	fmt.Println("Hello, world")
}

 $ ./testx
 fish: './testx' terminated by signal SIGKILL (Forced quit)

Basically, just using cgo and linking against sdl2 causes the crash:
package sdl

// #cgo linux freebsd darwin pkg-config: sdl2
import "C"

If you make the above file the whole go-sdl package, the binary will still crash.

Uhm it looks like lldb is even unable to spawn the binary:
(lldb) run
error: error: ::posix_spawnp ( pid => 24893, path = '/Users/rasky/Sources/go/src/ndsemu/testx/testx', file_actions = 0x7fff555225e8, attr = 0x7fff555225f8, argv = 0x7fd1e92a8150, envp = 0x7fd1e92ad520 ) err = Cannot allocate memory (0x0000000c)

so it seems like the binary is being linked in a way that the OS refuses to run it.
/cc @ianlancetaylor

just the same, cgo make it crash

same here but with github.com/shirou/gopsutil/cpu instead of sdl2.
trying to run the binary with dtruss gives Unknown error.

I'll notice that all Go binaries (including working ones) generate the -67062 error, but I think that's normal because they are not codesigned. The errors I get only when running these crashing cgo binaries is:
default	14:32:47.099750 +0200	taskgated	UNIX error exception: 3
default	14:32:47.099853 +0200	taskgated	no signature for pid=28180 (cannot make code: UNIX[No such process])

@rasky
On my environment (MacOS 10.12.14, Xcode8.3), non-cgo go programs do not generate -67062 errors. In my tests I performed, this error was strictly correlated to (1) crashes with the "signal: killed" message (2) cgo programs.
Additionally, I also get the "UNIX error exception: 3" message in the system logs

getting signal: killed when trying to use go run *.go
go build works, but then i get "Killed: 9" when I try to run the app

same here, dowloaded Xcode 8.2.1, replaced in /Applications and it works again..

For everyone running into this issue, I've been able to build binaries by adding -ldflags -s during the go build phase.

By default on Darwin the linker will invoke dsymutil to put the debug info directly into the executable.  This invocation of dsymutil is disabled by the linker's -s option.  Perhaps this is really a bug in dsymutil, or in how dsymutil deals with files created by the Go linker.  Could somebody with an affected system please try modifying the code in Link.hostlink in cmd/link/internal/ld/lib.go to skip invoking dsymutil, and see if that fixes the problem?  Thanks.

@ianlancetaylor it does.

The call to dsymutil was added to fix #8973.  I suppose the next question is: if we don't call dsymutil, what happens when you try to debug a Go program built with cgo?

I'm seeing this without using cgo, what's the suggested work around - use xcode 8.2?

@cosnicolaou, see above (https://github.com/golang/go/issues/19734#issuecomment-289822644). Does that not work for you?

thanks, it works, from reading the comments I thought this was a cgo only problem/fix. It's also painful for our build/automation for obvious reasons.

would adding these flags to build scripts have any negative repercussions that you're aware of? performance implications or anything like that?

no, it's just a pain to find all of the invocations etc.

agreed :) thanks for verifying they are safe to add though

oh, and all of our reg tests that build go programs also have to changed to work with this... that's a lot of code for us. When do you expect to have a 'transparent' fix done/out? Thanks.

I don't have a Mac.  I'm waiting for an answer to my question about #8973.

I've never used gdb/lldb on a Go program, so it's hard for me to spot patterns that are different from "usual". Is there some specific test that you would like to be performed (e.g.: specific things that you want us to check whether they work or not after dsymutil is taken out), or is it a really a generic question and it requires someone familiar with the matter that can do some sessions and find out if there's something wrong?

It's really a generic question, thanks.

I haven't tested it thoroughly yet but stripping a cgo binary also seems to work.

For building binaries, https://github.com/golang/go/issues/19734#issuecomment-289822644 is the way to go.
How do you go about go test-ing things though, without the signal: kill error?

@kchristidis The same, just do go test -ldflags -s ...

I suppose the next question is: if we don't call dsymutil, what happens when you try to debug a Go program built with cgo?

@ianlancetaylor, using which debugger?

@shurcooL I have no idea.  I don't use MacOS.  I'm just trying to help people connect the dots from #8973 to this problem, and trying to understand whether we can just stop calling dsymutil or whether that will cause #8973 to reappear.  #8973 mentions both gdb and lldb.

@ianlancetaylor Built 1.8 from source, commenting out the dsymutil section, ran gdb on a simple program that uses cgo, got this:
Reading symbols from $HOME/go/simple...
warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_amd64.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_context.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_darwin_amd64.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_libinit.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_setenv.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build096684628/runtime/cgo/_obj/gcc_util.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build185537422/mypackage/_obj/_cgo_export.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-build185537422/mypackage/_obj/mypackage.cgo2.o': can't open to read symbols: No such file or directory.

warning: `/var/folders/84/p363xh452_b1_f909jm5yw780000gn/T/go-link-879290217/go.o': can't open to read symbols: No such file or directory.
(no debugging symbols found)...done.

Quick and dirty workaround: have a "fake" dsymutil before /usr/bin/dsymutil in your path
$ ln -s /usr/bin/true /usr/local/bin/dsymutil
(of course, this will prevent you from using gdb)

I let my Mac update to Xcode 8.3 and now various binaries are killed during all.bash. So consider this reproduced. I'll try to figure it out, since I can't get any work done anymore.

CL 38747 should fix the problem (by not using dsymutil's output), and we'll get that backported into Go 1.8.1. Until then use -ldflags=-s as the workaround.
I filed #19772 to track a non-workaround fix. If that ends up being very simple we might consider it for a backport, but I'm not holding my breath.

in case anyone else ran into this, the go install -ldflags -s doesn't work. You have to go build -ldflags -s and then run that binary.

If anyone has a working Xcode 8.2 system, can you please run
cd go/src/runtime/testdata/testprogcgo
go build

and then post the 'testprogcgo' binary that is built? Here is fine if you can get Github to accept it; anywhere else is fine too. Thanks.

@rsc, testprogcgo.gz

Be the bot you want to see in the world.

I used https://developer.apple.com/download/more/?name=Xcode to download and install Command line tools 8.2 as work around. Remove /Library/Developer/CommandLineTools before installing.

For any gb users who stumble on this, you can apply the same fix from the comment above, except I need to add in the equal sign:
gb test -ldflags=-s

Thank you for the workaround. I was totally stuck until I looked in my update log and noticed XCode had updated yesterday.
Kill signal 9

thanks god ..

I was able to get around this while keeping the latest Xcode installed.  Download Xcode 8.2 from Apple and unpack it if your browser hasn't already.  Rename the extracted bundle, I renamed it to Xcode8.2 then copy it over to /Applications.  From a terminal use xcode-select to set 8.2 as toolchain to use.
sudo xcode-select --switch  /Applications/Xcode8.2.app/

@nicksnyder go install -ldflags -s does work for me but only if I delete the binary in my bin folder first.

@jekriske-lilly That's right, go install doesn't include the gcflags or ldflags when deciding something is out of date. We're planning to change that at some point, but for now that's how it is.

Cherry-picked.

I ran into this problem as well. Simply deleting Xcode 8.3 and replacing it with Xcode 8.2.1 did not downgrade the CommandLineTools. I needed to first remove /Library/Developer/CommandLineTools before installing Xcode 8.2.1. Another (probably better) option is to remove only /Library/Developer/CommandLineTools and download only the 8.2.1 CommandLineTools. See comment by @l0n3r4n83r above. Here is the link again: https://developer.apple.com/download/more/?name=Xcode

Apologies for the commit spam in this thread, I forgot GitHub would do that 😓. Are there any plans to backport the fix beyond 1.8? I can seem to reproduce this using this test code right back to at least 1.5.

At present there are no plans to make any more releases of anything earlier than 1.8.  We'll have to see how it goes.

There are no plans to backport this to Go 1.7 or earlier. Per https://golang.org/doc/devel/release.html#policy, Go 1.7 is no longer supported, since Go 1.8 is released.
If you must still use Go 1.7, the workaround is to compile with -ldflags=-s or to sudo ln -s /usr/bin/true /usr/local/bin/dsymutil, assuming /usr/local/bin is ahead of /usr/bin on your $PATH.

@rsc, I thought in #19069 we decided to support the past two releases. Isn't that what your 86abfbb931312db022ed8437a89219791ced6435 says?

I have been assuming that takes effect with Go 1.8: Go 1.9 - the first release with that policy - won't invalidate Go 1.8; Go 1.10 will.

Well, as one datapoint, Kubernetes still hasn't moved from Go 1.7 to Go 1.8, so this means you can't run Kubernetes (even Kubernetes 1.6) on a Mac now.

Er, build, not run. Maybe that's okay.

Right, build, and even then only build with newer Xcode. It's not like the gettimeofday crashes.

This may impact their point release builds, but I'm not sure how Kubernetes does their releases (on an actual Mac, or cross-compiled).
Running file on https://storage.googleapis.com/kubernetes-release/release/v1.6.1/bin/darwin/amd64/kubectl just says kubectl: Mach-O 64-bit executable x86_64.

They can always add -ldflags=-s or update to Go 1.8.1 like everyone else. Also, kubectl at least doesn't appear to have any cgo code in it (otool -l kubectl | grep nundefsym).

Yeah, kubectl seems to be cross compiled (it doesn't use the macOS DNS resolver).

none of the above workarounds worked for me :(

Update to Go 1.8.1, it is fixed now.

Go 1.8.1 Works! 🎉

use  go1.8.1 fixed it  good

authors: DomT4 OSMeteor abh aprilchild bradfitz cosnicolaou crazytyper dmaclach drewlandis favadi fighterlyt ianlancetaylor indie21 jekriske-lilly johnsogg josharian kchristidis l0n3r4n83r lyonel margerum nicksnyder rasky repustate rgeronimi rodrigo-brito rossor rsc samus shurcooL thejacobtaylor travissanderson-wf viru vyorkin


text:
regexp: needless allocations when compiling onepass regex

What did you do?
I ran benchmark of my program which contains compiling onepass regex and noticed that we  can specify onePassProg.Inst capacity.
https://github.com/golang/go/blob/73912a1b914251d29eb60c8e85262d931f895b90/src/regexp/onepass.go#L222-L228
What did you expect to see?
Allocation by append() will be reduced by specifying cap.
What did you see instead?
cap is not specified and it leads needless allocation.
System details
go version go1.8 linux/amd64
GOARCH="amd64"
GOBIN="/home/haya14busa/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/haya14busa"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build567867684=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.8 X:framepointer
uname -sr: Linux 4.4.48-1-MANJARO
LSB Version:	n/a
Distributor ID:	ManjaroLinux
Description:	Manjaro Linux
Release:	16.10
Codename:	Fringilla
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.24, by Roland McGrath et al.
gdb --version: GNU gdb (GDB) 7.12.1

Also, go version devel +73912a1 Mon Mar 27 23:36:36 2017 +0000 linux/amd64
I'll send a CL soon.

Good, the Go regex engine does need improvements. Its current performance is too weak.


text:
testing: document cap the number of concurrent goroutines with -race

What version of Go are you using (go version)?
1.9.2
GOOS=linux
GOARCH=amd64
What did you do?
package main

import (
	"math/rand"
	"strconv"
	"testing"
	"time"
)

func TestGoRoutines(t *testing.T) {
	for i := 0; i < 1<<15; i++ {
		t.Run(strconv.Itoa(i), func(t *testing.T) {
			t.Parallel()
			time.Sleep(time.Duration(rand.Intn(500000000)))
		})
	}
}

saved as x_test.go and run with go test -race
What did you expect to see?
The tests to pass. Instead,
race: limit on 8192 simultaneously alive goroutines is exceeded, dying

As I understand it, this is likely a technical limitation on Go's race detector. However, it would be nice for the testing package to be aware of this and, if raceenabled, cap the number of parallel tests at something below the 8192 limit (say, 7000?). Perhaps a better way would be a warning, but I know Go doesn't really like warnings.

I don't think is possible.  There is already a limit on the number of tests that are run in parallel, controlled by the -test.parallel option, whose default is runtime.GOMAXPROCS(0).  It doesn't help, though, because by the time we know that the test is going to run in parallel we have already started it.  We can't not start a test, because then we would never finish running all the non-parallel tests.
I'm going to close this as impossible but I'd be willing to hear a proposal for how to implement  it.

@ianlancetaylor fair enough. Could this be a doc fix, though? It was confusing the first time I stumbled upon it. I mean, the docs give an example (which I quoted above) that could easily be used to hit the 8,192 limit.

I'm OK with a doc fix but I'm not sure where it should go.  Maybe in https://blog.golang.org/race-detector ?

Throwing a couple ideas out there:

A comment under theTestGroupedParallel example that warns against using too many parallel tests if race is enabled. Perhaps something like, "Note: because it can be easy to run many parallel tests, care should be taken when the -race flag is toggled, as Go's race detector will kill the program if more than 8,192 goroutines are run simultaneously."

A // BUG(...) comment which warns against it, either in testing or race. (But, it's not really a bug per se...)

An addendum to the blog post is a good idea, but I've a feeling most people won't see it. (Could be mitigated if the text race: limit on 8192 simultaneously alive goroutines is exceeded, dying is added verbatim, since people will likely copy and paste that into Google.)

I'm fine with a comment in TestGroupedParallel, want to send a pull request?

@ianlancetaylor yessir. I'll get to it tonight. Do you want it inside TestGroupedParallel or just above/below it?

I would guess below it, actually.

@ianlancetaylor great that's where I added it. And darn, I should've tried making it with the new GitHub deal instead of git-codereview ;)

authors: ericlagergren ianlancetaylor


text:
runtime/race: no goroutines fatal error when running programs under race detector

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.10 darwin/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="/Users/nhooyr/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nhooyr/Programming/gopath"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7b/218sfv615xxf_w9ttnpjt0_r0000gn/T/go-build006239443=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
Run the following program:
package main

import "runtime"

func main() {
	runtime.Goexit()
}

You'll get:
fatal error: no goroutines (main called runtime.Goexit) - deadlock!

runtime stack:
runtime.throw(0x106c653, 0x36)
	/usr/local/Cellar/go/1.10/libexec/src/runtime/panic.go:619 +0x81
runtime.checkdead()
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:4148 +0x31b
runtime.mput(0xc42006a000)
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:4519 +0x49
runtime.stopm()
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1945 +0xba
runtime.findrunnable(0xc420024a00, 0x0)
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:2410 +0x50c
runtime.schedule()
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:2536 +0x13b
runtime.mstart1(0x0)
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1232 +0x9e
runtime.mstart()
	/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1188 +0x76
exit status 2

If you run with the race detector instead, then the runtime will not complain about there being no goroutines. I think that's because the race detector is running its own goroutines to detect races. I think the runtime should differentiate between them so that the race detector does not hide such serious errors.

I believe this is a duplicate of a very old issue. The resolution on the previous issue was the deadlock detection is best effort, not guaranteed.

On 16 Mar 2018, at 13:07, Anmol Sethi notifications@github.com wrote:
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.10 darwin/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="/Users/nhooyr/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nhooyr/Programming/gopath"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7b/218sfv615xxf_w9ttnpjt0_r0000gn/T/go-build006239443=/tmp/go-build -gno-record-gcc-switches -fno-common"
What did you do?
Run the following program:
package main
import "runtime"
func main() {
runtime.Goexit()
}
You'll get:
fatal error: no goroutines (main called runtime.Goexit) - deadlock!
runtime stack:
runtime.throw(0x106c653, 0x36)
/usr/local/Cellar/go/1.10/libexec/src/runtime/panic.go:619 +0x81
runtime.checkdead()
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:4148 +0x31b
runtime.mput(0xc42006a000)
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:4519 +0x49
runtime.stopm()
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1945 +0xba
runtime.findrunnable(0xc420024a00, 0x0)
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:2410 +0x50c
runtime.schedule()
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:2536 +0x13b
runtime.mstart1(0x0)
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1232 +0x9e
runtime.mstart()
/usr/local/Cellar/go/1.10/libexec/src/runtime/proc.go:1188 +0x76
exit status 2
If you run with the race detector instead, then the runtime will not complain about there being no goroutines. I think that's because the race detector is running its own goroutines to detect races. I think the runtime should differentiate between them so that the race detector does not hide such serious errors.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

/cc @dvyukov

The same happens if you import "C", right?

I think this might be sort of related to an issue that I am going to work on in the next couple of days https://github.com/golang/go/issues/21576. I started working on it with what seemed like a seemingly straightforward fix, but alas when cgo is enabled seems like an os.Notify like loop is started and won't catch deadlocks either. /cc @ianlancetaylor

We can fix cgo, we can fix signal.Notify, but then a program imports some package that starts a single background goroutine and we have the same problem. The buitin deadlock detector is really for the simplest handbook cases.

There is a difference between a background goroutine that could actually be doing something vs one that is deadlocked. If deadlocked, its a definite bug and we should detect it if possible.

Duplicate of #20588

authors: andybons davecheney dvyukov nhooyr odeke-em


text:
x/crypto/poly1305: implement a subset of the hash.Hash interface

Currently the interface to poly1305, func Sum(out *[TagSize]byte, msg []byte, key *[32]byte), forces the user to copy everything they want to sum into a single slice (msg). It would be nice to have a way to append multiple slices and then generate the sum.
The main motivation would be to avoid an allocation in the x/crypto/chacha20poly1305 AEAD. We could avoid adding new API surface by implementing this in something like internal/poly1305 but I thought I'd create this proposal to see if we could just add it to the poly1305 package. I'm imagining the API would be used something like (not tested):
zeros := [16]byte{}
h := poly1305.New(key)
h.Write(additionalData)
if len(additionalData) % 16 != 0 {
     h.Write(zeros[:16 - (len(additionalData) % 16)] // pad to 16
}
h.Write(plaintext)
if len(plaintext) % 16 != 0 {
     h.Write(zeros[:16 - (len(plaintext) % 16)] // pad to 16
}
lengths := [16]byte{}
binary.LittleEndian.PutUint64(lengths[0:8], uint64(len(additionalData))
binary.LittleEndian.PutUint64(lengths[8:16], uint64(len(plaintext))
h.Write(lengths[:])
h.Sum(out[:len(plaintext)])

For reference this is the current code:
https://github.com/golang/crypto/blob/613d6eafa307c6881a737a3c35c0e312e8d3a8c5/chacha20poly1305/chacha20poly1305_generic.go#L31-L39
Also, @aead's poly1305 library already implements [EDIT: a subset of] hash.Hash if you want to take a look: https://github.com/aead/poly1305.

I completely agree with Filippo here. That's exactly the reason why I did not implement hash.Hash for poly1305. IIRC this was already part of the chacha20poly1305 work - but was not really needed after Vald's  combined assembler implementation.

Poly1305 is a MAC and not Hash, so the naming would be misleading (although there is precedent in HMAC), it can't be reused with the same key, so Reset would be pointless, and can't be reused after calling Sum as it would be a key reuse.
I don't think hash.Hash is a good fit. Just a simple API with io.Writer and MAC([]byte) []byte would do.

SGTM. I think I'd use the function name Sum() to match HMAC (that would allow people to make their own MAC interface and switch between the two), but I don't really mind.

IIRC this was already part of the chacha20poly1305  - but was not really needed after Vlad's combined assembler implementation.

Yeah, I'm hoping this and the chacha20 API changes make it possible to get good performance without the complexity of the combined assembly approach.

I think I'd use the function name Sum() to match HMAC

Sure. But panic on anything after it is called once.

Yeah, I'm hoping this and the chacha20 API changes make it possible to get good performance without the complexity of the combined assembly approach.

You have all my support to make that happen BTW. Reducing the need for and amount of assembly is an explicit goal/wishlist item of mine. Let me know if you need anything from my side in that sense.

This is still missing the arm and s390x implementations.

I think CL 219057 was the final part of this so closing the issue.

authors: FiloSottile aead mundaym