fix(starport/services/chain): change tag and commit info when starport build

[fly33499]

I request to change tag and commit info when run starport chain build.
My testing scenario is here.

git tag
v0.1.0
v0.1.1
v0.2.0
v0.2.1
v0.2.3

starport chain bulild

// and run binary from chain build result. [always return first tag in the tag list.]

./mytest version
0.1.0

// [commit hash is just tag hash, not target commit when I see on the github]
./mytest version --long
name: mytest
server_name: mytestd
version: 0.1.0
commit: 8907357d75807bcc426d0de97d8ce82008ac40b1

If I wanted to make tag v0.2.3, I used "git tag-d" command to erase the rest of the tag T.T.
And I was confused because the tag hash was different from the information on github.

So, from my point of view, I have modified the code like this and I am going to make a full request for this part. (added comment )

// chain.go from starport/services/chain 182 lines

func (c *Chain) appVersion() (v version, err error) {
	repo, err := git.PlainOpen(c.app.Path)
	if err != nil {
		return version{}, err
	}

       // I've change repo.Tags to repo.TagObjects() for getting valid target commit hash.
	tags, err := repo.TagObjects()
	if err != nil {
		return version{}, err
	}

	var lastTag *object.Tag = nil

       // for get last tag info, use Foreach
	err = tags.ForEach(func(t *object.Tag) error {
		if t == nil {
			return errors.New("nil Tag exist in the TagObjects")
		}

		lastTag = t
		return nil
	})

	if err != nil {
		return version{}, err
	}

       // change tag hash to tag target hash.
	if lastTag != nil {
		v.tag = strings.TrimPrefix(lastTag.Name, "v")
		v.hash = lastTag.Target.String()
	}

	return v, nil
}

The code was modified to conduct a build test, and the desired result was obtained.
Please consider and check my opinion.

[fadeev]

Thanks for the PR, @fly33499! We'll review it shortly 👍

[Pantani]

@fly33499 thanks for the contribution

I made some tests here, and it's not working. I created a simple example to understand whats happened, and it seems not working too. Can you check your implementation, please?

Here is my example based on yours:
https://gist.github.com/Pantani/8118918fc76782a3d3d19f37f0b51765

[barriebyron]

thanks for this contribution @fly33499
I'm our technical writer at Tendermint and would be up for reviewing the associated docs changes to help our users understand this fix.

[ilgooz]

Hey @fly33499 thank you for taking a lead on this!

For an overkill solution, I think we should implement the equivalent of the following, programmatically, without depending on the git binary.

• HASH: git rev-parse HEAD

Hash is the hash of the latest commit in the currently checked out branch. (not the hash number of tag)

• TAG: git describe --tags

Always returns a semantic version. Similar to hash, it's relative to the currently checked out branch. It can output in two different formats:

v0.17.3: if there are no new commits added after tag is created.

v0.17.3-92-g48a13cc2: if there are new commits added after the tag is created. for this case, 92 is the number of commits created after v0.17.3 tag. And 48a13cc2 is the short version of the latest commit's hash.

I think this is the most precise way of determining both HASH and TAG. Specially for TAG, in case user may forget creating a new tag before running starport build, TAG output will show the exact semantic version by including the commit count and latest commit hash. Otherwise, it would always be showing the latest tag in the repo.

@fly33499 please let me know what do you think on this.

[ilgooz]

I think it's nice to keep the tests tho, but we need to figure out what's that funny error msg is about.

[ilgooz]

@fly33499 maybe we can create a new pkg as starport/pkg/repoversion and create a func called Determine(path string) (Version, error) and move the implementation inside appVersion() to there?

then appVersion() can use the repoversion.Determine() func under the hood.

so we can keep the chain.go simple.

[fly33499]

@fly33499 maybe we can create a new pkg as starport/pkg/repoversion and create a func called Determine(path string) (Version, error) and move the implementation inside appVersion() to there?

then appVersion() can use the repoversion.Determine() func under the hood.

so we can keep the chain.go simple.

Got it. I've just added and commit again. Could you check again? I've learned a lot of things from this project. Thanks a lot!!

[ilgooz]

LGTM! 🍻

[fly33499]

Thank you for review

[ilgooz]

Merged ~ thank you @fly33499!