Ensure excluded GDExtension files are not included in extension_list.cfg

[markeel]

The implemented solution to the problem of the error message appearing when an excluded GDExtension in an export of a project, is to filter the lines in the extension_list.cfg file to only include those that are in the paths actually included for export. If there are no entries remaining, don't write the file at all.

• Bugsquad edit, fixes: Export of GDExtension files that were excluded still appears in extension_list.cfg resulting in errors during startup #97207

[markeel]

I implemented what seems like a straightforward way to address this error message, by filtering out the files in the extension_list.cfg file during the process of exporting. This seems pretty low risk, but I welcome more eyes on the change. I tested with both a project with only one extension (which was excluded), and one with two extensions (one of which was excluded) per the minimal reproduction projects supplied in the issue.

[dsnopek]

Thanks!

I didn't test this, but looking at the code, this seems like a good approach overall :-)

I only have the one note below...

[dsnopek]

Thanks! The latest changes look good to me. I haven't re-tested, but I don't think anything has changed functionally since the last time I tested

[Repiteo]

Thanks! Congratulations on your first merged contribution! 🎉

[KoBeWi]

Does not work for me. I excluded all extension's files with EditorExportPlugin and it's still included in the extension list and prints errors on launch.
Looks like this only covers the case where files are excluded via the Export dialog.

[markeel]

Does not work for me. I excluded all extension's files with EditorExportPlugin and it's still included in the extension list and prints errors on launch. Looks like this only covers the case where files are excluded via the Export dialog.

I definitely didn't test with EditorExportPlugin, only the traditional Export dialog, so there must be something else when you are using that class. I didn't even know about that class, so that's a bit of new learning for me. I'm curious though, so I'll see if I can see what that class does differently. Do you have an example EditorExportPlugin I could test with?

[KoBeWi]

Check this comment: godotengine/godot-git-plugin#77 (comment)

[markeel]

I looked at this and see that the list of paths to process is not updated if a EditorExportPlugin uses the skip() method to drop it from the export list. I'll write a separate issue and see if there is a way to generalize this.

If you are are using the EditorExportPlugin just to exclude the git plugin, the easier way is to just use the export resource options and select the directories without using a plugin.

[markeel]

I created the following issue: #99698
I have a relatively simple update to the original change in #97207 that should address this as well.

[markeel]

Well, after doing some additional testing on my "fix" I realized that there were other changes in 4.4 that make supporting an editor export plugin excluding a GDExtensionPlugin like the Git plugin pointless. I closed issue: #99698

The root problem is that there are two export plugins competing for control of the same files, and there is no guarantee of the order they will run in, and the GDExtensionPlugin has now implemented additional warnings without the knowledge of any other editor export plugins running.

Oh well.

[akien-mga]

I haven't looked into the details, but I just want to make sure we're not adding workarounds to solve a symptom instead of handling the root problem properly.

The Git plugin is obviously a type of extension that nobody would want included in exports. So the extension system needs to have support for extension authors to register their extensions as editor only / no export, and this should be used for the Git plugin.

End users shouldn't have to manually exclude its files from the export presets.

[markeel]

Well that is the current strategy to prevent the files for an addon that is only for the editor from being in the exported package when they export their game. That is independent of whether it had a GDExtension or not. In the case where there was an GDExtension there was an extra twist to keep out just the line for that unwanted extension from extension_list.cfg.

Are you suggesting to add an option into plugin.cfg that would describe this plugin as being for the editor only?

That seems like it would make all the user's lives considerably easier.

[dsnopek]

The Git plugin is obviously a type of extension that nobody would want included in exports. So the extension system needs to have support for extension authors to register their extensions as editor only / no export, and this should be used for the Git plugin.

I think adding some sort of "no export" key to .gdextension files could make sense! However, there is some overlap between that and the ideas about global extensions that have been going around. Once we support global extensions, the Git plugin would almost certainly become one. So, I'd want to be careful not to add a key in such a way that would cause conflicts or confusion later.

[markeel]

The Git plugin is obviously a type of extension that nobody would want included in exports. So the extension system needs to have support for extension authors to register their extensions as editor only / no export, and this should be used for the Git plugin.

I think adding some sort of "no export" key to .gdextension files could make sense! However, there is some overlap between that and the ideas about global extensions that have been going around. Once we support global extensions, the Git plugin would almost certainly become one. So, I'd want to be careful not to add a key in such a way that would cause conflicts or confusion later.

Sounds like this discussion should be tied into whatever proposal is tracking this potential feature. In the meantime people will just have to manually exclude this type of addon. Is there a link to the global extension proposal?

[dsnopek]

Is there a link to the global extension proposal?

Here's the proposal but I don't think it captures all of the discussion that has gone on around this