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 git manifest to facilitate debug #323

Closed
ananos opened this issue Feb 15, 2019 · 8 comments · Fixed by #392
Closed

add git manifest to facilitate debug #323

ananos opened this issue Feb 15, 2019 · 8 comments · Fixed by #392

Comments

@ananos
Copy link

ananos commented Feb 15, 2019

It would be nice if we could add some kind of manifest for the commit hash, and print it out on solo5's preamble.

This came up while working on the aarch64 port of rumprun/solo5. We could add that on rumprun [nabla-containers/rumprun/issues/17] without having to touch solo5, but I suspect that would be potentially useful for other unikernel/port attempts, so maybe we could split it and add the current commit on each one of the layers.

What we had in mind is something along the lines of the following rough patch (for hvt & spt).

diff --git a/bindings/hvt/start.c b/bindings/hvt/start.c
index 0d224b0..960bc8f 100644
--- a/bindings/hvt/start.c
+++ b/bindings/hvt/start.c
@@ -37,6 +37,7 @@ void _start(void *arg)
     log(INFO, "  __|  _ \\  |  _ \\ __ \\\n");
     log(INFO, "\\__ \\ (   | | (   |  ) |\n");
     log(INFO, "____/\\___/ _|\\___/____/\n");
+    log(INFO, "Solo5: git: %x\n", VERSION);
 
     mem_init();
     time_init(arg);
diff --git a/bindings/spt/start.c b/bindings/spt/start.c
index e58eed2..07edc91 100644
--- a/bindings/spt/start.c
+++ b/bindings/spt/start.c
@@ -34,7 +34,7 @@ void _start(void *arg)
     log(INFO, "  __|  _ \\  |  _ \\ __ \\\n");
     log(INFO, "\\__ \\ (   | | (   |  ) |\n");
     log(INFO, "____/\\___/ _|\\___/____/\n");
-
+    log(INFO, "Solo5: git: %x\n", VERSION);
     mem_init();
     block_init(arg);
     net_init(arg);
diff --git a/configure.sh b/configure.sh
index 77ef415..0602698 100755
--- a/configure.sh
+++ b/configure.sh
@@ -217,6 +217,9 @@ case $(uname -s) in
         ;;
 esac
 
+VERSION=`git rev-parse --verify HEAD --short`
+HOST_CFLAGS="$HOST_CFLAGS -DVERSION=0x'${VERSION}'"
+
 cat <<EOM >Makeconf
 # Generated by configure.sh, using CC=${CC} for target ${TARGET}
 BUILD_HVT=${BUILD_HVT}

Please let us know your thoughts!

thanks!

@ricarkol
Copy link
Collaborator

Thanks @ananos, I agree with how useful something like this is. Although I would extend it by printing the version on both the tender and bindings side, as they have to match in version (not always, but ideally).

@hannesm
Copy link
Contributor

hannesm commented Feb 16, 2019

Thanks for this idea, I really like it. I have some additional thoughts on this:

  • (a) it should be exposed as command line parameter as well (i.e. solo5-hvt/solo5-spt --version)
  • (b) for solo5 releases (i.e. tarballs), it should return the released version (i.e. 0.5.0)
  • (c) for git versions, I'd suggest some more information:
    • (c1) was the repository dirty (check whether git status --porcelain output any lines)
    • (c2) include latest release and number of commits since then (using git describe --always HEAD --tags)
  • (d) for hvt include the list of compiled-in modules

The output I have in mind is something like: v0.4.1-20-f39104fd-dirty, v0.5.0, v0.4.1-50-g4530164. This allows to evaluate comparisons such as version >= v0.4.1 in a simple manner (which is not obvious from the git commit itself).

I agree with @ricarkol that both the tender and the bindings site should report their versions individually (and the bindings should expose this information via API so a unikernel can retrieve this information and report it in a structured way).

@mato
Copy link
Member

mato commented Feb 18, 2019

(d) for hvt include the list of compiled-in modules

This is already done in --help, so can be lifted into a --version.

The output I have in mind is something like: v0.4.1-20-f39104fd-dirty, v0.5.0, v0.4.1-50-g4530164. This allows to evaluate comparisons such as version >= v0.4.1 in a simple manner (which is not obvious from the git commit itself).

+1 on this output format.

I also agree that both bindings and the tender should report their version. Also, as an implementation detail, the version should go into a version.h that is generated by the Makefiles and then included by bindings and the tender, not as an extra -D onto CFLAGS.

and the bindings should expose this information via API so a unikernel can retrieve this information and report it in a structured way

What do you imagine this information to be used for by the unikernel? Also, when you say "structured way", what do you have in mind?

@hannesm
Copy link
Contributor

hannesm commented Feb 18, 2019

What do you imagine this information to be used for by the unikernel?

I dump the used (OCaml) packages and version numbers on startup of a unikernel via syslog. I'd like to add the bindings version number there as well. But re-thinking this could as well be the opam version of solo5-bindings-YYY.

Also, when you say "structured way", what do you have in mind?

The API I had in mind would return a string. As a user of that API I intend to include it in a (structured, RFC 5424) syslog message.

But as mentioned above, there's no need for that API, I can just use the opam version number.

@mato
Copy link
Member

mato commented Feb 18, 2019

The API I had in mind would return a string. [...] But as mentioned above, there's no need for that API, I can just use the opam version number.

Ok. A const char *solo5_version(void) API returning the string as defined above seems reasonable, we can add it separately once the information is available to the bindings via a version.h as I described above.

@ananos
Copy link
Author

ananos commented Feb 19, 2019

totally agree -- we can sketch something soonish and submit a wip pull request.

Just one note.

I also agree that both bindings and the tender should report their version.

Can you guys please clarify the above because I'm clearly missing something fundamental. In which case the bindings and the tender can be of a different version?

@mato
Copy link
Member

mato commented Feb 19, 2019

Can you guys please clarify the above because I'm clearly missing something fundamental. In which case the bindings and the tender can be of a different version?

At the moment, they can't. Or at least it's not supported and generally not expected to work. So, the motivation for adding --version to the tender is the same, ensure that when debugging you can be sure that you're using the matching tender with your bindings.

@ananos
Copy link
Author

ananos commented Feb 19, 2019

perfect, OK, we can start working on that and follow up with the proposed approach.

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

Successfully merging a pull request may close this issue.

4 participants