Obsolete (git) Merge Request #16: Fix 477, 478, 479, support MacOS in bootstrap, add git hash into version (merged)

Zdenek, this is the refined set of fixes. It is based on the master branch of the T-Platforms fork, but is cleaner. The commit with git hash was tailored to work even when git (or repo info) is not available . The fixes, formatting fixes and re-fixes were squashed together as appropriate. Additional care has been taken to not introduce new warnings. T-Platforms related changes have been completely dropped.

Please consider merging this.

[Zdenek Styblik]

Hello Alexander,

+               printf("No Additional Custom Mfg. (0x%02x)\n", len);

I believe word "fields" is missing in this message as it's present in all other messages. Also:

+                  "Additional Custom Mfg. field\n", *board_length);

probably should be "fields" instead of "field" given plural form is used in messages and source code.

Thank you,
Z.

[Alexander Amelkin]

I agree on the first one. Although that is just a move of the message from the original code (see hunk 4 of the patch, original file's line 2806). I initially fixed that wording, but then decided to leave the original message intact. You see in in the diff just because diff format can't properly handle such moves (moved to another code block and indented). Do you want me to fix this?

I disagree on the second one. It's a continuation of "ERROR: File has insufficient data (%d bytes) for the", and is meant to say that we expected data for a (i.e. single) custom field (found a corresponding type/length byte previously) but the actual file was too short.

[Zdenek Styblik]

I agree on the first one. Although that is just a move of the message from the original code (see hunk 4 of the patch, original file's line 2806). I initially fixed that wording, but then decided to leave the original message intact. You see in in the diff just because diff format can't properly handle such moves (moved to another code block and indented). Do you want me to fix this?

Yes please. Please, squash if possible.

I disagree on the second one. It's a continuation of "ERROR: File has insufficient data (%d bytes) for the", and is meant to say that we expected data for a (i.e. single) custom field (found a corresponding type/length byte previously) but the actual file was too short.

I see. Ok.

[Alexander Amelkin]

Done. Squashed.

[Alexander Amelkin]

Zdenek,

In addition to the changes already in merge request, I have a proposal on how to further improve versioning for git-based builds.

Here is the commit log for that new change in my development branch:

The previously introduced mechanism of generating
a git revision was only using the abbreviated hash.
That approach doesn't give a clue whether version
1.8.18.2c99bf6 is newer or older than 1.8.18.7f8d374.

The project uses tags, so `git describe` can be
employed to produce an incremental revision number
since the last tag. Version 1.8.18.13 is much more
understandable and comparable. However that doesn't
answer the question "what codebase was used". To
address that, the abbreviated hash should also be
preserved. Hence, this commit introduces a new
versioning scheme like `1.8.18.13.gee01aa5`.

For git snapshots when git program is absent the
version will be like `1.8.18.0.gsnapshot`.

For cases when .git directory is missing (Release
compilation?) the suffix part will be omitted
completely yielding a version like `1.8.18`.

The suffix generation has been moved to the added
csv-revision script.

You can see the patch at https://sourceforge.net/u/spirit-rc/ipmitool/ci/6b84ccd

Please tell me what you think and whether I should add that to the existing merge request?

Alexander.

[Zdenek Styblik]

Hello Alexander,

can we handle this perhaps in a separate MR? However, I would like to have a go on that shell script and write in a portable way which Bash isn't, unfortunatelly.

Thank you,
Zdenek

[Alexander Amelkin]

What would you suggest as a portable solution?

[Zdenek Styblik]

Good evening Alexander,

to put the code where my mouth is:

#!/bin/sh
set -e
set -u

git_output=$(git describe --first-parent --tags 2>/dev/null || printf "error")
git_rev=$(printf -- "%s" "${git_output}" | cut -d'-' -f2)
git_hash=$(printf -- "%s" "${git_output}" | cut -d'-' -f3-)

if [ "${git_output}" != "error" ] && [ -n "${git_rev}" ]; then
    printf -- ".%s.%s\n" "${git_rev}" "${git_hash}"
elif [ -d "./.git" ]; then
    printf ".0.gsnapshot\n"
else
    printf ".unknown\n"
fi

Best regards,
Zdenek

[Alexander Amelkin]

Zdenek, please check https://sourceforge.net/u/spirit-rc/ipmitool/ci/f0cd51d
I have replaced bash with sh, but didn't use your approach with set -eu as it requires at least XSI POSIX extension. I got rid of the <() bashism and -a option in [] (also an XSI extension). This version seems to be absolutely pure POSIX.

[Zdenek Styblik]

Hello Alexander,

not a fan, but I have no case against it :) Feel free to do MR.

Zdenek

[Zdenek Styblik]

Thank you for MR and fixes.

Zdenek