nervosnetwork / ckb

The Nervos CKB is a public permissionless blockchain, and the layer 1 of Nervos network.
https://www.nervos.org
MIT License
1.15k stars 228 forks source link

fix: fix empty git commit version of cli #4629

Closed xG0Ian closed 1 week ago

xG0Ian commented 2 weeks ago

What problem does this PR solve?

Issue Number: close #4550 nervosnetwork/ckb-cli#609

Problem Summary: When no git information was found, the sub-command --version outputs a pair of empty brackets.

What is changed and how it works?

What's Changed: String::from_utf8(r.stdout) will always return a string when stdout is empty. https://github.com/nervosnetwork/ckb/blob/2fb17ab8f4c3a5bd005cbf726e897e3201258bc9/util/build-info/src/lib.rs#L106-L124 https://github.com/nervosnetwork/ckb/blob/2fb17ab8f4c3a5bd005cbf726e897e3201258bc9/util/build-info/src/lib.rs#L129-L141

So extra_parts.is_empty() is always return false https://github.com/nervosnetwork/ckb/blob/2fb17ab8f4c3a5bd005cbf726e897e3201258bc9/util/build-info/src/lib.rs#L95

unwrap it with default value "", check if they are empty

let commit_describe = self.commit_describe.clone().unwrap_or("".to_string());
let commit_date = self.commit_describe.clone().unwrap_or("".to_string());
if !commit_describe.is_empty() || !commit_date.is_empty() {
    write!(f, " ({})", extra_parts.as_slice().join(" "))?;
}

Related changes

Check List

Tests

Side effects

Release note

None: Exclude this PR from the release note.
Title Only: Include only the PR title in the release note.
Note: Add a note under the PR title in the release note.
eval-exec commented 2 weeks ago

Thank you,

The ckb --version in the ckb release includes the commit hash, but the ckb-cli does not. I think there might be an issue with the ckb-cli build process, where the git commit hash is not being injected into the ckb-cli binary.

https://github.com/nervosnetwork/ckb/releases/tag/v0.118.0-rc1

image

xG0Ian commented 2 weeks ago

Yes, this PR is only for the problem mentioned in those two open issues.

eval-exec commented 1 week ago

Hello, I'm unsure if there's an issue with the package workflow in ckb-cli, as it doesn't appear to be injecting CKB CLI's Git commit information (https://github.com/nervosnetwork/ckb-cli/blob/develop/.github/workflows/package.yaml).

Need to determine why the CKB CLI (ckb-cli) does not display its Git commit hash when running the command ckb-cli --version.

xG0Ian commented 1 week ago

@eval-exec Hi Eval,

Yes, I’m quite sure the issue is that CKB CLI's Git commit information wasn’t injected, which is why it shows ckb-cli xxx ( ) with empty parentheses ( ).

The change in this PR is to fixing where we don’t have Git commit info by ensuring that do not display ( ). However, we still need to inject the Git commit hash into ckb-cli. I will open an issue to ckb-cli and fix it if I can.