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

Add customization for git ignore file #100

Closed
wants to merge 1 commit into from

Conversation

oliverlee
Copy link

Define hedron_compile_commands_setup to perform installation configuration. Values set during this step remain constant across invocations of refresh_compile_commands.

This commit introduces a single install config parameter ignore_file. If not set, .gitignore is used. This can be replaced with the local, unversioned ignore file .git/info/exclude.

@cpsauer
Copy link
Contributor

cpsauer commented Dec 28, 2022

Hey, @oliverlee! Thank you for this sophisticated effort to improve things.

I'd love to understand a little more what your "needs backtrace" is here. What higher level goals does this resolve for you?

(What's going on in my head as I read:
I can see the (clever) way of setting configuration across rules via the workspace setup fn. Cool :)
I was guessing the root goal came from a repo where you can't check in code related to this tool for some reason--but then this probably wouldn't have provided enough benefit to justify its complexity, since you'd still have to maintain unresolved changes in WORKSPACE anyway, which is the file that presumably changes more frequently.
[and some minutia, only worth our discussing after the above])

Cheers,
Chris

@oliverlee
Copy link
Author

Hi. We have source code spread out across many project repositories. In order to reduce duplication, we have a single tooling repo that brings in linting/formatting/testing/etc. related dependencies - the WORKSPACE of a project repository is fairly small.

https://github.com/hedronvision/bazel-compile-commands-extractor is getting brought in via tooling. We'd like to avoid additional steps in tooling dependencies (in this case, updating .gitignore) since this is only used by some but not all devs.

Define `hedron_compile_commands_setup` to perform installation
configuration. Values set during this step remain constant across
invocations of `refresh_compile_commands`.

This commit introduces a single install config parameter `ignore_file`.
If not set, `.gitignore` is used. This can be replaced with the local,
unversioned ignore file `.git/info/exclude`.
@cpsauer
Copy link
Contributor

cpsauer commented Jan 13, 2023

Hey guys! Sorry for being slow--got a little buried in tasks over here. Resurfacing. I'm so sorry; I promise I'm usually faster, like with my initial replies.

After some more thought, I think the move here is to just move over to .git/info/exclude for everyone. I don't see any big downsides compared to the current, and had I known about it when I originally wrote this, I think I should have chosen to do to do it that way, handling more complexity automatically for users. (Conversely, if you think any of that is wrong, please do say!)

I'll take a shot at that shortly.

Thanks for teaching me something new!
(To try to return the favor--and in case we need workspace configuration again--some quick ideas that might simplify this pattern: I'm guessing repr() might be able to simplify the json writing fn, and having a first round of templating the code directly from the workspace rule might be able to avoid it altogether.)

Cheers!
CS

cpsauer added a commit that referenced this pull request Jan 13, 2023
Pointed out in #100
(the other changes buildifier would propose are, I think, a net negative to readability)
cpsauer added a commit that referenced this pull request Jan 13, 2023
This makes ignoring work automagically for people, while minimizing the code changes they have to think about or check in. #100 and #59 are examples of use cases that this simplifies. It also marginally simplifies the case where people can't commit use of this tool to the repo they're working on.
IMO tools should to do this more broadly, especially now that git is so dominant.
Hidden gitignore documented in https://git-scm.com/docs/gitignore
@cpsauer
Copy link
Contributor

cpsauer commented Jan 13, 2023

Sweet! Done in the commits backlinked above. Thanks for a great PR to build on.
(Closing, but with the PR's soul firmly in the codebase.)
Please do let me know whether that works for you!

@cpsauer cpsauer closed this Jan 13, 2023
@oliverlee
Copy link
Author

Thanks for incorporating the changes! I think the simpler approach makes sense.

As for workspace configuration, that may be useful in the future - I've seen some projects use external as a directory containing BUILD files for non-Bazel external dependencies. In those situations, it would be useful to customize the name of the external symlink (e.g. bazel-external or .external).

@cpsauer
Copy link
Contributor

cpsauer commented Jan 13, 2023

Great :)
Re external: That one we've discussed a lot. Last I checked, I think having a package named external caused conflicts inside bazel's execution sandbox--but maybe it's fine for just repository rules. It does greatly simplifies the implementation of this plugin to have the workspace match the structure of the execution sandbox (which uses external and bazel-out). More details in #30, if you're curious

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

Successfully merging this pull request may close these issues.

None yet

2 participants