Skip to content

Fix reverse: flag keep#967

Merged
mvdan merged 2 commits intoburrowers:masterfrom
fakeboboliu:master
Aug 7, 2025
Merged

Fix reverse: flag keep#967
mvdan merged 2 commits intoburrowers:masterfrom
fakeboboliu:master

Conversation

@fakeboboliu
Copy link
Contributor

This fixes issue caused by 6ac80db.

reverse exits if we have a package that won't build with empty build constraints.

@luantak
Copy link
Member

luantak commented Aug 4, 2025

Can you explain exactly what this fixes

@fakeboboliu
Copy link
Contributor Author

Can you explain exactly what this fixes

consider a main.go:

//go:build sometag

func main() {
    return
}

garble reverse -tags=sometag . will shows build constraints exclude all Go files.

@luantak
Copy link
Member

luantak commented Aug 4, 2025

@fakeboboliu
Copy link
Contributor Author

Can you add a regression test for this to:

https://github.com/burrowers/garble/blob/master/testdata%2Fscript%2Freverse.txtar

done.

before:

> go test -run Script/reverse
 ......
            > exec ./tag.bin
            [stderr]
            main.NMSpPYY2n3
            > cp stderr tag.stderr
            > ! grep "main.CallerFuncName" tag.stderr
            > stdin tag.stderr
            > exec garble reverse -tags sometag ./case/tag
            [stderr]
            build constraints exclude all Go files in $WORK\case\tag
            [exit status 1]
            FAIL: testdata\script\reverse.txtar:54: unexpected command failure
 ......

after:

> go test -run Script/reverse
PASS
ok      mvdan.cc/garble 11.716s

Copy link
Member

@mvdan mvdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mvdan
Copy link
Member

mvdan commented Aug 7, 2025

Pretty embarassing that I didn't notice the mistake in my "simplification" refactor.

@mvdan mvdan merged commit 1b9c305 into burrowers:master Aug 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants