Feature/metadataapi

[andydotxyz]

Add support for reading metadata through API.
More work to follow, including tests. Marked draft for discussion.

Fixes #1940 (indirectly)

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.
• Public APIs match existing style.

[lucor]

Just a small typo, apart that LGTM

[Jacalz]

This looks very promising to me. I know that this is a draft but I just had two questions inline.

[coveralls]

Coverage increased (+0.04%) to 61.874% when pulling a1112e3 on andydotxyz:feature/metadataapi into 8dd874b on fyne-io:develop.

[andydotxyz]

I cannot figure out these test failures. They are passing correctly on my fork but failing here (and pass on my desktop too).
The report is specifying line numbers that don't exist (400 something on package_test.go that has just 180).
Also in my checkout / fork the test "Test_PackageWasm" does not appear, yet that is failing.
Anyone got an idea what is happening?

[Bluebugs]

I cannot figure out these test failures. They are passing correctly on my fork but failing here (and pass on my desktop too). The report is specifying line numbers that don't exist (400 something on package_test.go that has just 180). Also in my checkout / fork the test "Test_PackageWasm" does not appear, yet that is failing. Anyone got an idea what is happening?

This is very weird, but I have the impression that it is actually merging with develop and running the tests from develop. If you do a manual merge locally, you should be able to see them too.

[andydotxyz]

I cannot figure out these test failures. They are passing correctly on my fork but failing here (and pass on my desktop too). The report is specifying line numbers that don't exist (400 something on package_test.go that has just 180). Also in my checkout / fork the test "Test_PackageWasm" does not appear, yet that is failing. Anyone got an idea what is happening?

This is very weird, but I have the impression that it is actually merging with develop and running the tests from develop. If you do a manual merge locally, you should be able to see them too.

Hmm, I thought I had done so, but maybe a day or two back was not enough ;)

[andydotxyz]

This is very weird, but I have the impression that it is actually merging with develop and running the tests from develop. If you do a manual merge locally, you should be able to see them too.

Amazing, this was bang on thanks. Fixed :)

[Jacalz]

I'll try to have a look at this later today or tomorrow.

[Bluebugs]

This might be a silly idea, but could address @Jacalz concern. What do you think of having a "fyne metadata" command line that read the metadata and generate a go file instead. This could be used then by go generate. This would make go build actually work and provide metadata too.

[Jacalz]

Can you elaborate a bit more in how you intend it to be used and how it is different from fyne bundle?

[andydotxyz]

This might be a silly idea, but could address @Jacalz concern. What do you think of having a "fyne metadata" command line that read the metadata and generate a go file instead. This could be used then by go generate. This would make go build actually work and provide metadata too.

Yes we could do this. However metadata can change (version numbers etc) and forgetting to run this would mean the version info is out of date. Also exposing the App.Metadata() API will not be possible / reliable if the packaging of this data is optional...

[Bluebugs]

Can you elaborate a bit more in how you intend it to be used and how it is different from fyne bundle?

By reading its information from the FyneApp.toml and generating the proper metadata structure. Very different job compared to fyne bundle, I think.

[Bluebugs]

However metadata can change (version numbers etc) and forgetting to run this would mean the version info is out of date. Also exposing the App.Metadata() API will not be possible / reliable if the packaging of this data is optional...

I agree there is a synchronization problem. fyne package could maybe run go generate to make sure it is in sync, but for someone using go build, they would have to do it manually and thus the risk of not being in sync. Neither are ideal solution and both have drawback. I am personally fine with both as long as there drawback are documented.

[andydotxyz]

However metadata can change (version numbers etc) and forgetting to run this would mean the version info is out of date. Also exposing the App.Metadata() API will not be possible / reliable if the packaging of this data is optional...

I agree there is a synchronization problem. fyne package could maybe run go generate to make sure it is in sync, but for someone using go build, they would have to do it manually and thus the risk of not being in sync. Neither are ideal solution and both have drawback. I am personally fine with both as long as there drawback are documented.

Ah cunning, combine the best of both. Inject latest into generated source code so go build still works. That sounds genius and may be possible. It might require another API because the app would have to inject the data instead of using the compiler. Will have to think about this more to see if it can be done more cleanly.

[andydotxyz]

This might be a silly idea, but could address @Jacalz concern. What do you think of having a "fyne metadata" command line that read the metadata and generate a go file instead. This could be used then by go generate. This would make go build actually work and provide metadata too.

I looked into this further and unfortunately I don't think it's practical. If the developer forgets to commit the file, and they use the go build then the compile will fail. With the ldflags approach in this PR it will not fail as the values fall back to default / "in development" info.

[Jacalz]

I’m genuinely sorry but I would still vote for keeping the icon part out for now and implementing it at a later date. That way we can get more time to think about a better solution that doesn’t have the current drawbacks.

This API will be great even without the icon. I think you've done a great job with it. It kind of falls in line with the extra build metadata that was added for Go 1.18 (that might even be useful for this in the future).

[andydotxyz]

Info for the 3 different builds, "basic" is just go build, "bundled" adds the icon using fyne bundle and then "meta" uses this PR. I measure the time from start to main executing and from main executing to window entering foreground. This is with a 512x512 icon, 84kB.

type	size	start time	show time
basic	-	61ms	238ms
bundle	+82kB	56ms	237ms
meta	+115kB	59ms	238ms

Although the source code is much smaller (no bundle file, but even if there was the MD5 is half the size of the bundled byte array source) the compiled image data is around 2% smaller bundled (compared with image size) vs around 30% larger than the source image due to the MD5 storage format.
However you can see that startup time is pretty much the same.

I could look at different string encodings for the image data I guess.

[andydotxyz]

Just a quick reminder of the current scanario:

• v2.1.4 and "go build": User does no icon bundling and no custom icon is set on a window or in the app bundle
• v2.1.4 with bundled icon and "go build": The users code will set the window icon but most OS will not show this for app icon.
• v2.1.4 with fyne package: OS will show app icon but based on OS the window icon may still be blank
• v2.1.4 with fyne package and bundled icon: Apps and windows will all have icon set.

As you can see above it basically requires user adding code into the app and bundling the icon manually to get it everywhere.

• This PR and "go build": icon is empty as before
• This PR and bundled icon with "go build": as above - as before, partial
• This PR and "fyne package": Icon will appear in all places without extra code
• This PR and "fyne package" with bundled icon: as before, all OK
• New possibility with this PR and "fyne build" the icon is included by default through FyneApp.toml or -icon param

The potential downside is that users bundling their own icon manually may have it stored twice when packaging for the benefit of controlling it's manual inclusion when not packaging and using "go build" only.

I can see that this is a tradeoff, but I think the lowered inconvenience for the primary target (packaging) is worth it for the possible downside during development (avoiding fyne tools and finding the icon not inserted unless code added).

[Jacalz]

I think you are right. There is no better way around this. I don't know if it's worth it, but I guess we could consider using https://github.com/cristalhq/base64 if it makes any difference?

Another though that I had was if it makes sense to have an option to turn off the metadata? Something like a nometadata build tag that ignores loading it in and parsing it.

[andydotxyz]

I tried some other encodings but they just didn't work (either invalid for a ldflags command line parameter, or much larger than comparison).
In the process I think I found that my previous measurements were not correct. So this shows updated size comparison:

Comparing this time to metadata included - but no icon metadata:

• base64 + 98k
• hex - 163k

So base64 is actually less than half the increase than I though it was compared to the efficient bundling of bytes directly (which was +82k). It makes me feel a little better.

[Jacalz]

Looks great. This is a really nice improvement. It looks like this branch isn't updated with the fyne buildcommand but I guess that this will work it as well.

[andydotxyz]

It didn't but I pushed an additional minor change over the top which added metadata to the serve command as well