Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meson and Flatpak build files have hardcoded versions, so now we hardcode it in three different files #286

Open
NikolajSchlej opened this issue Sep 2, 2022 · 16 comments

Comments

@NikolajSchlej
Copy link
Collaborator

The recently added Meson build file (thanks to @hughsie for it) has a hardcoded version field, that is PITA to update every time.
We currently use version.h as way to hardcode the version in a single place, but its current format is not really suitable for Meson to get the version string out of it.

@vit9696, @hughsie, @eli-schwartz, any ideas on how to improve version.h to be Meson-compatible?

@hughsie
Copy link
Contributor

hughsie commented Sep 2, 2022

The troll in me wants to suggest deleting everything except meson.build, but I'll save that suggestion for another day.

@NikolajSchlej
Copy link
Collaborator Author

Well, I'd counter-troll by suggesting deletion of meson.build itself and making the downstream built everything they want in a way they want, but that will also be both impolite and stupid, considering it was myself who merged the two PRs about it in the first place.

@hughsie
Copy link
Contributor

hughsie commented Sep 2, 2022

https://stackoverflow.com/questions/59201214/can-the-meson-project-version-be-assigned-dynamically has a nice list; but I did wonder how keen you are on the AXX version scheme; i.e. would a systemd-style date version number (e.g. 20220902) be best all round?

@eli-schwartz
Copy link

The usual options are:

  • have the build system define the version, and configure version.h.in to version.h, replacing @VERSION@ with the actual version
  • write a small python script that parses the header and extracts the version, use this as project(..., version: run_command(....).stdout().strip())
  • write a release script that does some automated work for tagging a new release, including bumping the version number in multiple places (both the build system and the header file)

@hughsie
Copy link
Contributor

hughsie commented Sep 2, 2022

In all my projects I do the former, i.e. the .h file has a @Version@ config.h thing -- but I think @NikolajSchlej probably wants to leave the old buildsystem as untouched as possible.

@NikolajSchlej
Copy link
Collaborator Author

The current version scheme is a mess and I'm not really keen on holding onto it.
Back in 2013, when this whole thing started, I've used a major.minor.bugfix versioning schema.
2013

Then came end of 2015 when I decided that the current parsing/rebuilding engine code is broken enough to warrant a full rewrite, and started doing it, slowly but surely. However, until the new builder is ready, I could not ditch the "original" or "old engine" codebase, so instead I made a "new engine" branch and started calling the tools based on it "New Engine Alpha N", with N being incremental number.

Fast forward to September 2022, the "old engine" is still not ditched because the fancy new builder is still not ready, and the "Alpha N" is now "A61" already.

I'm fine to choose any other kind of versioning schema, including returning to major.minor.bugfix or major.minor.bugfix.build_date, but I do not think it's terribly important to do soon enough. Need to finish that stupid builder first...

@NikolajSchlej
Copy link
Collaborator Author

Yet another file with a version, appdata.xml, no now we have three files to update. This is slowly getting worse.

@eli-schwartz
Copy link

That was added in #299 for flathub. I strongly suspect that flathub will just use meson anyway, but, the current state of affairs is that appdata.xml isn't even installed, when it should be. By a build system.

I would argue that the original PR implementing this was buggy. It should have configured the file from within meson using meson.project_version() to single source the version number, and do something similar for version.h if anyone expresses interest in installing flathub metadata using other build systems.

Then it would use install_data() to place that file into the installation tree where flathub expects it.

... also the file is named incorrectly, the appstream standard soft deprecated "appdata" in favor of "metainfo".

Fortunately @hughsie knows rather a lot about this topic, so I'll leave the rest to him...

@vulpes2
Copy link
Contributor

vulpes2 commented Sep 25, 2022

Hi, I'm the one who opened the PR at flathub, it would be great if we can settle on one single build system across the project. While I'm aware that we have meson and cmake available, neither of them were beneficial in their current state. The cmake configuration wasn't set up to generate a meaningful make install target, so I had to copy the binaries manually anyway. The meson config doesn't build UEFITool, only the other two CLI utilities, which also isn't useful for this particular use case.

That was added in #299 for flathub. I strongly suspect that flathub will just use meson anyway, but, the current state of affairs is that appdata.xml isn't even installed, when it should be. By a build system.

Flatpak-builder supports both meson or cmake, we just need to settle on one and make it work.

Yet another file with a version, appdata.xml, no now we have three files to update. This is slowly getting worse.

IMO we need some kind of script to write these version info in the appstream metadata automatically before making a new release so we don't have to worry about updating it manually anymore.

@NikolajSchlej
Copy link
Collaborator Author

it would be great if we can settle on one single build system across the project

Too many people want too many different things, all equally viable, so at this point it's easier for me and @vit9696 to just add some metadata to please yet another build system and move on. The project started with a single build system: qmake, but then (as Qt went LGPLv3 out of the blue) I had to switch to something Qt-less for CUI tools that did not require Qt (and provide reimplementations of Qt classes used extensively by the engine, like QByteArray, QTreeItem and so on). That something was CMake, and it proved to be alright for all major OSes and compilers. Then @hughsie came with Meson files, then @vulpes2 came with Flathub ones, now we are here with 4 different build systems and no easy way out of it.

The cmake configuration wasn't set up to generate a meaningful make install target, so I had to copy the binaries manually anyway.

Make an issue about it, maybe, so we can fix it in CMake files? I don't really understand what's the problem there, but I'm not using Linux outside of VMs, so my knowledge of it is limited.

IMO we need some kind of script to write these version info in the appstream metadata automatically before making a new release so we don't have to worry about updating it manually anymore.

Indeed, and I'll need to bump priority of this slightly because the more metadata files we get into this repo, the harder it becomes to not forget to update version numbers in all of them.

@NikolajSchlej NikolajSchlej changed the title Meson build file has a hardcoded version Meson and Flatpak build files have hardcoded versions, so now we hardcode it in three different files Sep 25, 2022
@vulpes2
Copy link
Contributor

vulpes2 commented Sep 25, 2022

now we are here with 4 different build systems and no easy way out of it

flatpak-builder can call either meson or cmake so it's technically not an additional burden. If either one of them are fully functional, we wouldn't need to even call unixbuild.sh anymore when building the flatpak.

Make an issue about it, maybe, so we can fix it in CMake files? I don't really understand what's the problem there, but I'm not using Linux outside of VMs, so my knowledge of it is limited.

Sure, opened a new issue with more info: #300

@hughsie
Copy link
Contributor

hughsie commented Sep 26, 2022

That was added in #299 for flathub

The appdata (or metainfo, it really doesn't really matter) file isn't designed as a build config file, it's designed as history - e.g. look at fwupd (or pretty much any other GUI app) for a good example: https://github.com/fwupd/fwupd/blob/main/data/org.freedesktop.fwupd.metainfo.xml#L35 -- and then the release process involves writing the release notes there https://github.com/fwupd/fwupd/blob/main/RELEASE#L13 and then actually generating the NEWS file from the appdata file. The advantage of this is that we can show nice markup when the client supports it and it's all parsable my computers rather than trying to parse ChangeLog or NEWS files in all the different formats.

@hughsie
Copy link
Contributor

hughsie commented Sep 26, 2022

also, don't think you have to write the release notes this way, lots of apps just do <release version="1.8.5" date="2022-09-22"/> which just gives the number and release date -- which is what 90% of the things processing the appdata file care about.

@NikolajSchlej
Copy link
Collaborator Author

@hughsie, now I'm really confused, TBH. I don't want to put history anywhere but on GH release page, and the file we have on FlatHub now is indeed a build configuration of sorts:
https://github.com/flathub/com.github.LongSoft.UEFITool/blob/master/com.github.LongSoft.UEFITool.yml

It also is certainly version-specific, i.e. has both a direct link to A61 tag, and a sha256 of source tarball.

Does this mean I need to manually update that file every time a new release is made?

@vulpes2
Copy link
Contributor

vulpes2 commented Sep 28, 2022

I don't want to put history anywhere but on GH release page

I agree, having one single source of release notes is the best option, everything else can take the content from from that source. Eventually I would like to address this by adding a github actions workflow to automatically pick up new releases and update the appstream metadata file in this repo, this way you won't have to worry about the release notes in the appstream metadata anymore.

It also is certainly version-specific, i.e. has both a direct link to A61 tag, and a sha256 of source tarball.

Don't worry about this, flatpak-external-data-checker can automatically pick up new releases and open PRs to update the URLs and hashes, with an option to automatically merge it if build is successful. Later this week I will update the manifest to enable this feature.

Edit: I did put it in and just forgot about it lol, it's already enabled

@hughsie
Copy link
Contributor

hughsie commented Sep 29, 2022

I agree, having one single source of release notes is the best option

This is what I do for fwupd releases; write the XML metainfo content and then convert it to a NEWS file automatically, and then use the NEWS markdown content for the github release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants