serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Everywhere: Refactor version reporting so that the Kernel is the one source of truth

Open kleinesfilmroellchen opened this issue 2 years ago • 6 comments

There's quite some stuff in this PR but it basically amounts to:

  • Get rid of the three distinct version information sources there are right now (Core::Version::SERENITY_VERSION, /res/version.ini, uname syscall).
  • Make the uname syscall the one source of truth for version information.
  • Bake the actual version information into the Kernel; get rid of /res/version.ini.
  • Make uname(1) (the utility) POSIX-compliant by reporting the git commit hash with -v.

image

Fixes #15071

kleinesfilmroellchen avatar Aug 29 '22 12:08 kleinesfilmroellchen

Embedding git revisions in binaries means that two identical source trees except for a comment change somewhere produce different binaries.

The cool way to handle this is to have deterministic builds and print the hash of the build outputs, not the inputs 😛

nico avatar Aug 29 '22 15:08 nico

two identical source trees except for a comment change somewhere produce different binaries.

In my mind that's an acceptable drawback / feature ;^), but I also understand the desire for reproducible builds. The problem of using the build output hash is that it's basically meaningless, compared to being able to trace a build back to a specific commit in the serenity repo.

linusg avatar Aug 29 '22 15:08 linusg

Embedding git revisions in binaries means that two identical source trees except for a comment change somewhere produce different binaries.

As long as you're not committing, the binaries will be the same. Either way, I don't know the best approach forward, but to me, it feels beside the point for this specific PR. We can have an additional discussion about how there's a hard-coded version string that some parts of userland use while Core::Version can already read /res/version just fine, but again, besides the point for this PR, :yaksteps: :^) What do you think, @linusg ?

kleinesfilmroellchen avatar Aug 29 '22 17:08 kleinesfilmroellchen

it's basically meaningless, compared to being able to trace a build back to a specific commit in the serenity repo.

Au contraire! If your build is deterministic, the output hash clearly corresponds to one (or more! If there are multiple revs producing the same output) input rev.

nico avatar Aug 29 '22 23:08 nico

Sure, but unless someone will be keeping a list of build hashes mapping to their corresponding commit hashes, I'm not sure how I would determine which "version" of serenity I'm running just from that?

linusg avatar Aug 29 '22 23:08 linusg

Ah, so that's why you need it. I just went with the minimal POSIX APIs we need to have.

kleinesfilmroellchen avatar Aug 30 '22 11:08 kleinesfilmroellchen

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Sep 24 '22 18:09 stale[bot]

Is this still active?

circl-lastname avatar Sep 27 '22 08:09 circl-lastname

I think it is totally mergeable, yes.

kleinesfilmroellchen avatar Sep 27 '22 20:09 kleinesfilmroellchen