jcarr
/
git2go
1
0
Fork 0
Code Issues 52 Pull Requests 31 Packages Projects Releases Wiki Activity

Generate stringer files automatically #841

Merged
g4s8 merged 6 commits from 543-stringer-generate into main 2021-11-08 08:38:55 -06:00
Conversation 2 Commits 6 Files Changed 3 +29
g4s8 commented 2021-10-14 10:07:02 -05:00 (Migrated from github.com)
Copy Link

Added stringer annotations to git.go for ErrorClass and
ErrorCode. Added generate rule for Makefile to generate
string representations for these types (first building cgo files
in _obj dir to get C constants). Finally, updated ci actions
workflow to check that generated files are up to date.

Fixes: #543

Added `stringer` annotations to `git.go` for `ErrorClass` and `ErrorCode`. Added `generate` rule for `Makefile` to generate string representations for these types (first building cgo files in `_obj` dir to get C constants). Finally, updated `ci` actions workflow to check that generated files are up to date. Fixes: #543
lhchavez (Migrated from github.com) reviewed 2021-11-07 12:51:55 -06:00
lhchavez (Migrated from github.com) left a comment
Copy Link

yay, thanks for the CI addition!

yay, thanks for the CI addition!
.github/workflows/ci.yml
lhchavez (Migrated from github.com) commented 2021-11-07 12:50:40 -06:00
Copy Link
      run: git diff --quiet --exit-code || (echo "detected changes after generate" ; git status ; exit 1)

this way the caller will be able to see what were the changes.

```suggestion run: git diff --quiet --exit-code || (echo "detected changes after generate" ; git status ; exit 1) ``` this way the caller will be able to see what were the changes.
Makefile
lhchavez (Migrated from github.com) commented 2021-11-07 12:51:42 -06:00
Copy Link

is it possible to have this be

generate: static-build/install/lib/libgit2.a
	go generate --tags "static" ./...

? that way whenever a new stringer is added, the Makefile doesn't need to be updated (folks will forget about this, haha). the _gen_file function also depends a bit on implementation details that are not guaranteed to be true forever.

is it possible to have this be ```suggestion generate: static-build/install/lib/libgit2.a go generate --tags "static" ./... ``` ? that way whenever a new stringer is added, the Makefile doesn't need to be updated (folks will forget about this, haha). the `_gen_file` function also depends a bit on implementation details that are not guaranteed to be true forever.
g4s8 (Migrated from github.com) reviewed 2021-11-08 02:17:58 -06:00
Makefile
g4s8 (Migrated from github.com) commented 2021-11-08 02:17:58 -06:00
Copy Link

Unfortunately, go generate --tags "static" ./... will not work with CGO constants from libgit2 headers (as in diff.go and git.go), so it's a workaround to extract these constants from header files to .go files, see this question: https://stackoverflow.com/q/69566767/1723695

Unfortunately, `go generate --tags "static" ./...` will not work with CGO constants from libgit2 headers (as in `diff.go` and `git.go`), so it's a workaround to extract these constants from header files to `.go` files, see this question: https://stackoverflow.com/q/69566767/1723695
g4s8 commented 2021-11-08 02:20:43 -06:00 (Migrated from github.com)
Copy Link

@lhchavez thanks for review! Please check new changes and answers.

@lhchavez thanks for review! Please check new changes and answers.
lhchavez (Migrated from github.com) reviewed 2021-11-08 07:56:00 -06:00
.github/workflows/ci.yml
@ -108,0 +122,4 @@
git submodule update --init
sudo apt-get install -y --no-install-recommends libssh2-1-dev
go install golang.org/x/tools/cmd/stringer@latest
- name: Generate files
lhchavez (Migrated from github.com) commented 2021-11-08 07:55:26 -06:00
Copy Link
    - name: Install libgit2 build dependencies
      run: |
        git submodule update --init
        sudo apt-get install -y --no-install-recommends libssh2-1-dev
    - name: Generate files

oops, forgot this last round! otherwise make generate will fail because CGo won't be able to find git2.h: https://github.com/libgit2/git2go/runs/4139740720?check_suite_focus=true (note that this depends on the suggestion that was left in Makefile to work correctly).

```suggestion - name: Install libgit2 build dependencies run: | git submodule update --init sudo apt-get install -y --no-install-recommends libssh2-1-dev - name: Generate files ``` oops, forgot this last round! otherwise `make generate` will fail because CGo won't be able to find `git2.h`: https://github.com/libgit2/git2go/runs/4139740720?check_suite_focus=true (note that this depends on the suggestion that was left in `Makefile` to work correctly).
Makefile
lhchavez (Migrated from github.com) commented 2021-11-08 07:54:36 -06:00
Copy Link

oh that's likely because the static library wasn't correctly built / installed (that's why the generate: rule has static-build/install/lib/libgit2.a as a dependency, which fixes this issue). i just tried this on my machine and it worked. can you give this a try?

also, minor thing, can the generate rule be moved to just below the default: one? i just realized that it's included in the various library sections, and that was a bit confusing, haha.

oh that's likely because the static library wasn't correctly built / installed (that's why the `generate:` rule has `static-build/install/lib/libgit2.a` as a dependency, which fixes this issue). i just tried this on my machine and it worked. can you give this a try? also, minor thing, can the `generate` rule be moved to just below the `default:` one? i just realized that it's included in the various library sections, and that was a bit confusing, haha.
g4s8 (Migrated from github.com) reviewed 2021-11-08 08:10:39 -06:00
Makefile
g4s8 (Migrated from github.com) commented 2021-11-08 08:10:39 -06:00
Copy Link

Thanks, it's working now, updated in the latest commit

Thanks, it's working now, updated in the latest commit
lhchavez (Migrated from github.com) reviewed 2021-11-08 08:27:43 -06:00
.github/workflows/ci.yml
@ -108,0 +120,4 @@
- name: Install libgit2 build dependencies
run: |
git submodule update --init
sudo apt-get install -y --no-install-recommends libssh2-1-dev
lhchavez (Migrated from github.com) commented 2021-11-08 08:27:42 -06:00
Copy Link
        sudo apt-get install -y --no-install-recommends libssh2-1-dev
        go install golang.org/x/tools/cmd/stringer@latest

haha, stringer wasn't installed! (and we may need to tweak PATH unless GH does it for us)

```suggestion sudo apt-get install -y --no-install-recommends libssh2-1-dev go install golang.org/x/tools/cmd/stringer@latest ``` haha, `stringer` wasn't installed! (and we may need to tweak `PATH` unless GH does it for us)
g4s8 commented 2021-11-08 08:35:46 -06:00 (Migrated from github.com)
Copy Link

Exported GOPATH in 9c737a6d50

Exported `GOPATH` in https://github.com/libgit2/git2go/pull/841/commits/9c737a6d50a84f16b4387ddd109ef0fbf4a49b5c
lhchavez (Migrated from github.com) approved these changes 2021-11-08 08:38:41 -06:00
lhchavez (Migrated from github.com) left a comment
Copy Link

yay! now it works in CI!

thank you for this improvement :D

yay! now it works in CI! thank you for this improvement :D
Sign in to join this conversation.
Reviewers
No reviewers
lhchavez
Labels
Clear labels   
bug

   
duplicate

   
enhancement

   
invalid

   
question

   
up for grabs

   
wontfix
No Label
bug
duplicate
enhancement
invalid
question
up for grabs
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: jcarr/git2go#841
No description provided.
Delete Branch "543-stringer-generate"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?