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

Add uiVersion API #143

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add uiVersion API #143

wants to merge 2 commits into from

Conversation

cody271
Copy link
Contributor

@cody271 cody271 commented Sep 30, 2022

Fixes #141

@kojix2 kojix2 mentioned this pull request Sep 30, 2022
@kojix2
Copy link
Contributor

kojix2 commented Oct 1, 2022

Thanks.

I downloaded the shared library for Linux from GitHub Actions and tried it. There was a little strange behavior.
I wrote the following code and linked libui.so.

#include <stdio.h>
#include "./ui.h"

int main(void){
	const char *version = uiVersion();
	printf("%s\n", version);
	return 0;
}

The output is 4251607. This was unexpected. I expected uiVersion() to output the tag or short commit hash f77fae9a.
I was curious, so I got the uiVersion branch of the repository and built the shared library myself.
This time I got f77fae9a as expected.

I think this may not be the expected behavior. If so, I would like to fix it...

@kojix2
Copy link
Contributor

kojix2 commented Oct 1, 2022

4251607 is here.
Add-uiVersion-API-·-libui-ng-libui-ng-f77fae9
So I guess it will work fine once it is merged into the master?

@cody271
Copy link
Contributor Author

cody271 commented Oct 2, 2022

Wow, good find .. I don't understand why the GitHub Actions CI runner would insert its own custom git tag into the build!?

So I guess it will work fine once it is merged into the master?

Yes it should be fine once merged into master, but still this is very strange behavior. Merits further investigation.

@szanni
Copy link
Contributor

szanni commented Oct 2, 2022

I like the option of getting the current commit hash for displaying debugging information.

Apart from that I see limited usefulness for developers, as this is runtime and the hash is not incremental. Being able to probe for minimum API version would be more useful to me. I outlined how other popular libraries do this in the ticket.

I do see the usefulness of getting the hash though, maybe it would make sense to have both?

Compile time constants with version number (once the first release is tagged, we would probably want do increment these manually directly in the header) and a uiVersionHash or uiVersionGit that can be probed at runtime and where we use meson to keep track of things.

@kojix2
Copy link
Contributor

kojix2 commented Oct 3, 2022

I prefer a simpler approach. For example, "5.0.0-f77fae9a"

@cody271
Copy link
Contributor Author

cody271 commented Oct 4, 2022

I prefer a simpler approach.

As much as I'd like to keep this API very simple .. after thinking about it some more, the design requirements here are actually somewhat complex. Our version info API needs to support all these scenarios:

  • Runtime and compile-time
  • Semantic version and git commit hash
  • Machine parseable and string format
  • Build tool automated, not manually incremented

@matyalatte
Copy link
Contributor

Hi, I have an idea about the versioning API.
How about using calendar versioning like Ubuntu?

You can try this to see how it will work in meson.build.

git_date = run_command('git', 'show', '-s', '--format=%ci', check: true)
git_date_parsed = git_date.stdout().strip().substring(0, 10).split('-')
git_hash_short = run_command('git', 'rev-parse', '--short', 'HEAD', check: true)
message('year : ' + git_date_parsed[0])
message('month: ' + git_date_parsed[1])
message('day  : ' + git_date_parsed[2])
message('hash : ' + git_hash_short.stdout().strip())

Then, we can define constants like this.

  • UI_VERSION "yyyy.mm.dd.hash"
  • UI_VERSION_YEAR, UI_VERSION_MONTH, UI_VERSION_DAY, UI_VERSION_HASH
  • UI_VERSION_NUMBER yyyymmdd = UI_VERSION_YEAR * 10000 + UI_VERSION_MONTH * 100 + UI_VERSION_DAY

I think we don't need semantic version when the library have no tags in 5 years.
If you want to make tags, you can use dates like 2023.09

@matyalatte
Copy link
Contributor

I made a commit for the calendar versioning (matyalatte@291a474).

Before running git commit

// config.h
#define UI_VERSION "2022.09.29.f77fae9a"
#define UI_VERSION_YEAR 2022
#define UI_VERSION_MONTH 9
#define UI_VERSION_DAY 29
#define UI_VERSION_HASH 0xf77fae9a

After the commit

// config.h
#define UI_VERSION "2023.09.24.291a4740"
#define UI_VERSION_YEAR 2023
#define UI_VERSION_MONTH 9
#define UI_VERSION_DAY 24
#define UI_VERSION_HASH 0x291a4740

@matyalatte
Copy link
Contributor

I haven't tested it yet, but I found this.
https://github.com/actions/checkout#Checkout-pull-request-HEAD-commit-instead-of-merge-commit
So, we can use the HEAD commit in the workflow.

@cody271
Copy link
Contributor Author

cody271 commented Sep 26, 2023

Hi, I have an idea about the versioning API. How about using calendar versioning like Ubuntu?

While I'm not against adding calendar versioning as well, my original list of requirements would need to be added first.

@matyalatte
Copy link
Contributor

my original list of requirements would need to be added first.

What does "Runtime and compile-time" mean?
Does It mean the version info should be defined as constants in a header file, not only as the uiVersion* functions?

At least, the calendar versioning can meets the rest of your list, I think.
The version will be incremented by git commit.

@szanni
Copy link
Contributor

szanni commented Sep 26, 2023

What does "Runtime and compile-time" mean? Does It mean the version info should be defined as constants in a header file, not only as the uiVersion* functions?

Correct. See the requirements I posted here. What is missing here would possibly be something like UI_VERSION_HASH.

Runtime would essentially just be const char * uiVersion() { return UI_VERSION; } etc. More important to me as a library user are the compile time checks. Defines are essentially what all my favorite libraries use that IMO have sane coding practices.

I would personally voice my opinion against date versioning. I see little benefit for a library in doing so and it complicates things immensely. Dates give zero context on what the API provides. Will I face breaking changes when upgrading to a newer version? Semver solves this problem elegantly.
With regards to the suggested code: I'm also unsure which date would be used? Commit date? Merge date? Could this decrease the version number when pushing a rebased commit?

I see a lot of benefit in including the git hash for git based builds to identify possible bugs. For any API related stability questions git head should be considered best effort.

@cody271 regarding your PR/proposal in particular: what about packaged releases? This code will not give any meaningful version number if the git tree is not present. I know we are not distributing packages yet but to me it seems that distributing snapshots would be a major point for introducing version numbers?

Having though about this some more: We most likely need to separate git versioning and point release versioning entirely. Every new commit would otherwise need to incease the patch number or even minor if a new feature was added. Possible but painful with regards to constant merge conflicts. I would most likely vote for manual semver versioning with the ability to get a build hash (git commit hash) for debugging purposes.

@matyalatte
Copy link
Contributor

What is missing here would possibly be something like UI_VERSION_HASH.

So, you mean we should generate a hash when compiling, not only the commit hash?

I see little benefit for a library in doing so and it complicates things immensely. Dates give zero context on what the API provides. Will I face breaking changes when upgrading to a newer version? Semver solves this problem elegantly.

Well, to be honest, I also prefer the manual semver.
But if you want to automate versioning, calver is the simplest way to do.
And I don't think we need the numeric versioning like semver when the latest tag is five years old.

With regards to the suggested code: I'm also unsure which date would be used? Commit date? Merge date? Could this decrease the version number when pushing a rebased commit?

It'll get the last commit date via git. But it's just a suggession.
We can change the command if it has issues.

@matyalatte
Copy link
Contributor

Every new commit would otherwise need to incease the patch number or even minor if a new feature was added. Possible but painful with regards to constant merge conflicts.

I think we can increment the patch number with PRs, not for each commit.
It'll cause no conflicts because the versioning will be done on the master branch.
You can also use the merged event to automate it.

I made a repo as an example for the semver automation.
matyalatte/Auto-Semver-Template: Scripts to update semantic version with github actions

It can update version info when a PR is merged like this.
matyalatte/Auto-Semver-Template@e324432

@szanni
Copy link
Contributor

szanni commented Oct 4, 2023

What is missing here would possibly be something like UI_VERSION_HASH.
So, you mean we should generate a hash when compiling, not only the commit hash?

No, I actually meant something like UI_GIT_HASH. Build hash is out of the scope of this library I would say. I know some industries need it, but this is IMO something they can take care of themselves.

And I don't think we need the numeric versioning like semver when the latest tag is five years old.

Fair point. I understand your reasoning for calver now. I would personally hope that through the introductions of semver we would actually start tagging releases. All the current tags are still from libui.

It'll get the last commit date via git. But it's just a suggession. We can change the command if it has issues.

We would need to test what it does for rebased commits or merges from commits with a commit date in the past.

I think we can increment the patch number with PRs, not for each commit. It'll cause no conflicts because the versioning will be done on the master branch. You can also use the merged event to automate it.

I don't believe we can actually automate any of this. Some commit will want to increase the patch number, new features would however require an increase in minor number (and breaking ones major, at least once we reach 1.0.0).
I don't see a reasonable way of upgrading the changelog automagically either. So manual editing seems inevitable here to me as well.

Another thing to keep in mind are multiple build systems. See the build.zig PR #198.
I don't believe we should try to get too fancy around versioning. It is grunt work, like the changelog itself. We should just have a very clear defined, step by step process.

@matyalatte
Copy link
Contributor

Some commit will want to increase the patch number, new features would however require an increase in minor number

You can use PR labels to set which number should be incremented like this.
matyalatte/Auto-Semver-Template#2

I don't see a reasonable way of upgrading the changelog automagically either.

You can get merged commits, PR titiles, comments, etc. from the merged PR.
So, it's possible theoretically.

We would need to test what it does for rebased commits or merges from commits with a commit date in the past.

Right. We have to add another commit if we do that things.

Or using the merged date might be better?
We can generate a header file with merged PRs like this.

// ui_version.h
#define UI_VERSION "${merged_date}.${git_hash}"
#define UI_VERSION_YEAR ${merged_year}
#define UI_VERSION_MONTH ${merged_month}
#define UI_VERSION_DAY ${merged_day}
#define UI_VERSION_NUMBER ${merged_date}
#define UI_GIT_HASH "${git_hash}"

Then, we can use it with any build systems, and no need git for building.
We can also use a similar method for semver automation.

@rubyFeedback
Copy link

So in the last example, people would check against the constant
UI_VERSION_NUMBER typically? Nothing wrong with autogenerating
version-information; just wondering what people would use in
downstream code.

For instance, in ruby, while it is no standard at all, often the version
is kept in an uninspiringly called constant called ... VERSION. In my
own projects I stick to this, and sometimes I also add a method called
".version?" as slightly-nicer-to-read option, e. g. if the project is called
Foobar, then the ruby call for querying that version would simply be:

Foobar.version? # rather than Foobar::VERSION

And perhaps if one refers to kojix2' bindings then it may be:

LibUI::UI_VERSION_NUMBER # or whatever the constant will be that is then used

Perhaps I should also explain a bit why it is important for downstream
folks to know the version. Libui-ng adds new things every now and then, see
the Changelog:

https://github.com/libui-ng/libui-ng/blob/master/CHANGELOG.md

However had, when I update my own add-on code for libui-stuff,
I depend on:

a) libui-ng repository here, logically, but more importantly
b) kojix2' code making this available via ruby FFI/fiddle

And then I am also a bit slow sometimes, so I may be inactive
for, say, weeks. Then I try to catch up and in particular in this
situation, it is very important to know what I am already supporting
and covering, and what not. And, of course, what kojix2' is
supporting too - sometimes things are not yet stable or kojix2'
is testing things, so there may be pre-releases. Things may take
time.

This is why I think being able to pinpoint towards specific libui-ng
versions can be super-helpful. I can re-check things at a later time
to see whether they work now (if they did not work before), and
add code to help support new features in libui-ng (such as, hopefully,
tooltips one day \o/). Hope that explains the context a little for the
request.

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

Successfully merging this pull request may close these issues.

How to get a version number
5 participants