Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gazelle can modify a "# gazelle:ignore" BUILD file #266

Closed
treaster opened this issue Feb 8, 2017 · 4 comments
Closed

gazelle can modify a "# gazelle:ignore" BUILD file #266

treaster opened this issue Feb 8, 2017 · 4 comments

Comments

@treaster
Copy link

treaster commented Feb 8, 2017

Running gazelle to regenerate a BUILD file containing # gazelle:ignore can modify the existing file. It appears that the changes are only reordering/cleanup of the existing rules, with no structural changes. However, the docs say "# gazelle:ignore in a BUILD file will instruct gazelle to leave the file alone.", which isn't true.

One possible fix would be to update the documentation for accuracy.

Another possible fix would be to avoid writing out anything when the ignore directive appears.

I can see arguments for both the cleanup mode (the existing behavior) and the strict non-intervention mode (what the documentation says). I think either of my proposals would address the problem.

The ideal solution might be to use the strict mode by default, then also provide a --cleanup mode which executes the existing cleanup behavior.

To reproduce

mkdir -p gazelle_test/code
cd gazelle_test/
echo "package main\nfunc main() {}" > code/main.go
print "# gazelle:ignore\n\n\n# some comment" > BUILD.orig
print "# gazelle:ignore\n\n\n# some comment" > BUILD
gazelle --repo_root=. --go_prefix=code
diff code/BUILD.orig code/BUILD

produces:

3,4c3
< 
< # some comment
\ No newline at end of file
---
> # some comment

i.e. some whitespace was removed. Other changes are also possible, such as reordering of build rule attributes.

Random implementation thought

One possible approach to implementing the strict mode might be to use something like this in gazelle's main.go where MergeWithExisting() is called.
https://github.com/bazelbuild/rules_go/blob/master/go/tools/gazelle/gazelle/main.go#L101

f, err := merger.MergeWithExisting(f, existingFilePath)
switch err.(type) {
    case nil:
        // write out the generated file as usual
    case GazelleIgnoreErr:  // "# gazelle:ignore" was seen
        // cancel further processing of this BUILD, but continue
        // the loop to process other BUILDs.
        continue  
    case error:
        return err // interrupt the loop and cancel program execution          
}
@treaster
Copy link
Author

treaster commented Feb 24, 2017

I discovered that comments inside a srcs block will also be removed even when # gazelle:ignore appears in the file. For example:

# gazelle:ignore
...
srcs = [
    "foo.go",
    # "bar.go",
    "baz.go",
]

becomes

# gazelle:ignore
...
srcs = [
    "foo.go",
    "baz.go",
]

@pmbethe09
Copy link
Member

Seems we should fix or remove this feature -- I don't have any cycles right now, but could review someone else's changes.

@jayconrod
Copy link
Contributor

I'll take a look at this.

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Feb 28, 2017
Previously, Gazelle would avoid merging files with a "#
gazelle:ignore" comment, but it would still rewrite them. This could
result in small changes, e.g., whitespace.

Gazelle will no longer rewrite these files at all.

Fixes bazelbuild#266.
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Feb 28, 2017
Previously, Gazelle would avoid merging files with a "#
gazelle:ignore" comment, but it would still rewrite them. This could
result in small changes, e.g., whitespace.

Gazelle will no longer rewrite these files at all.

Fixes bazelbuild#266.
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Feb 28, 2017
Previously, Gazelle would avoid merging files with a "#
gazelle:ignore" comment, but it would still rewrite them. This could
result in small changes, e.g., whitespace.

Gazelle will no longer rewrite these files at all.

Fixes bazelbuild#266.
jayconrod added a commit that referenced this issue Mar 1, 2017
Previously, Gazelle would avoid merging files with a "#
gazelle:ignore" comment, but it would still rewrite them. This could
result in small changes, e.g., whitespace.

Gazelle will no longer rewrite these files at all.

Fixes #266.
@treaster
Copy link
Author

Thanks for making this fix, Jay!

groodt pushed a commit to groodt/rules_go that referenced this issue Mar 14, 2022
Add go mod

Also make the build docs nicer and cleanup the file directory to be in line
with most other go projects. This also make it so you can build jsonnet
without setting -o on go build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants