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

[Bug]: new gitignore module causes regressions in parsing #657

Closed
Aghassi opened this issue Mar 21, 2024 · 3 comments
Closed

[Bug]: new gitignore module causes regressions in parsing #657

Aghassi opened this issue Mar 21, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@Aghassi
Copy link

Aghassi commented Mar 21, 2024

What happened?

There are two issues I have had since updating aspect-cli from 5.8.5 to 5.9.19

  1. gitignores with **file.txt are noted as invalid (less of an issue)
  2. gitignore with just a folder name like generated_folder will not be ignored by gazelle even though it is ignored by git
  3. exclude syntax for git ignored module still causes things importing from that module to traverse the folder and generate files based on that import

Version

Development (host) and target OS/architectures:

Output of bazel --version:
6.4.0

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:

Language(s) and/or frameworks involved:

How to reproduce

Have a repo with a top level build file that sets up gazelle, but enables js generation in a subfolder only

Have a gitignore at the root of the repo and one in the subfolder. This means this gitignore extends the one above it.

Have an index file that imports from a folder that is generated by bazel via `write_source_file` which is gitignored. This provides IDE support but also generates files that are ignored by git into the source tree

Run gazelle and see the build file in the nested package update no matter what you do or where you run gazelle from

Any other information?

No response

@Aghassi Aghassi added the bug Something isn't working label Mar 21, 2024
@github-actions github-actions bot added the untriaged Requires traige label Mar 21, 2024
@jbedard
Copy link
Member

jbedard commented Mar 23, 2024

AFAICT ** not followed by a / is invalid? Try it in bash, or see the doublestar gomod docs as examples. But you're saying it works in a plain gitignore?

@jbedard
Copy link
Member

jbedard commented Mar 23, 2024

exclude syntax for git ignored module still causes things importing from that module to traverse the folder and generate files based on that import

Can you give an example of that? I'm not sure I understand the case...

@Aghassi
Copy link
Author

Aghassi commented Mar 25, 2024

AFAICT ** not followed by a / is invalid? Try it in bash, or see the doublestar gomod docs as examples. But you're saying it works in a plain gitignore?

It could very well be invalid syntax, but as far as I'm aware git won't throw or inform you so that could be on us. For that issue, I would expect the error message be very clear. EX

.gitignore at path /path/to/file has the following invalid syntax:
- **thing
- etc

Please remove or fix this syntax to remove this message. This ignore will be skipped for this run

To the second question, the issue I was having was I had a folder named styled-system which was ignored in .gitignore as styled-system, no modifiers. Git respects the ignore and ignores it. However, gazelle does not respect the ignore and tries to add those files to the BUILD file because we had a file which imported from this folder. So what I would expect is, given an import + and ignore, the ignore takes precedence and does not add the files to the BUILD file. Does that make sense?

@alexeagle alexeagle removed the untriaged Requires traige label Mar 27, 2024
@jbedard jbedard self-assigned this Apr 15, 2024
@jbedard jbedard closed this as completed Apr 17, 2024
alexeagle pushed a commit that referenced this issue Apr 22, 2024
Fix #657

---

### Type of change

- Bug fix (change which fixes an issue)

### Test plan

- New test cases added

GitOrigin-RevId: f8f960e20b10051a0bc27c0777108463c10e82c8
alexeagle pushed a commit that referenced this issue Apr 22, 2024
Fix #657

---

### Type of change

- Bug fix (change which fixes an issue)

### Test plan

- New test cases added

GitOrigin-RevId: f8f960e20b10051a0bc27c0777108463c10e82c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants