Skip to content

Add .gitignore file#659

Closed
kinkie wants to merge 6 commits intosquid-cache:masterfrom
kinkie:gitignore
Closed

Add .gitignore file#659
kinkie wants to merge 6 commits intosquid-cache:masterfrom
kinkie:gitignore

Conversation

@kinkie
Copy link
Contributor

@kinkie kinkie commented Jun 6, 2020

No description provided.

@kinkie
Copy link
Contributor Author

kinkie commented Jun 6, 2020

This iteration is specific to the tools I use; but it would make sense in my opinion to list here the files we never want to add to our sources, such as for SPONSORS

Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for trying to add a .gitignore file. I believe it is the right thing to do.

IMO, we should have a policy for which files to include. We should not have an official .gitignore without an official inclusion policy. My last attempt to develop such a policy has failed. See #29 for details. I am very open to better policy ideas.

Needless to say, you can have your own .gitignore. I know that it is not ideal. I have (more than) one.

.gitignore Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I am just curious: Why do you have that file or directory in a git repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't. The objective is to ensure we don't add it by mistake

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood, thank you!

FWIW, I do not think we should list files just because they should not be added to the repository. The list of such files is nearly infinite. Fortunately, policies suggested so far did not include such files IIRC.

Sorry for using words. I will try the suggested "iterate in code" approach from now on (for this PR) -- I noticed that suggestion after writing the above.

@kinkie
Copy link
Contributor Author

kinkie commented Jun 7, 2020

Thank you for trying to add a .gitignore file. I believe it is the right thing to do.

IMO, we should have a policy for which files to include. We should not have an official .gitignore without an official inclusion policy. My last attempt to develop such a policy has failed. See #29 for details. I am very open to better policy ideas.

I think we shouldn't obsess over a policy; a .gitignore file can very well be our policy. It is formally defined, both machines and humans can read it, it has a well understood review workflow, feels ideal to me.

At a very high level, I think a .gitignore file for the project could include the things we do not want to include and that right now we need to be mindful about. Specific tools configuration file (as in this case), and autogenerated files feels like a first good start.

None of this is necessary, but it can avoid boring review iterations by preventing trivial mistakes.

@rousskov
Copy link
Contributor

rousskov commented Jun 8, 2020

I think we shouldn't obsess over a policy

I agree. I think we should not obsess over anything.

a .gitignore file can very well be our policy.

I do not understand what you mean, especially in a context where my proposed additions to .gitignore have been effectively rejected in #29 . If you mean the "core developers can add whatever entries they want to be ignored in their setup" policy, then I doubt such a policy is a good idea, but we can use that as a starting point if you prefer.

FWIW, I have not heard any valid arguments against my original proposal to list all untracked files that can be generated in a working directory by Squid build command(s). I think we can expand that to typically-embedded configuration .files of common Squid development tools. That combination will take care of two out of three entries proposed in this PR (and I can add many more from my own .gitignore).

Needless to say, if you officially add your personal Squid .gitignore, I will have to abandon mine, and vice versa, so we should either find an inclusive official compromise or do nothing (officially). A middle-ground official solution would be worse than none here.

Specific tools configuration file (as in this case), and autogenerated files feels like a first good start.

I agree. The second part is what I proposed in #29 .

None of this is necessary, but it can avoid boring review iterations by preventing trivial mistakes.

I agree.

@kinkie
Copy link
Contributor Author

kinkie commented Jun 8, 2020

Second iteration tried. Feel free to push to this PR with any amendments you think are warranted, let's iterate in code instead of words

@kinkie kinkie requested a review from rousskov June 8, 2020 13:36
@rousskov
Copy link
Contributor

rousskov commented Jun 8, 2020

Feel free to push to this PR with any amendments you think are warranted, let's iterate in code instead of words

OK, please wait for my push.

rousskov added 4 commits June 8, 2020 14:21
I am using the following policy:

1. Any untracked file that can be generated in a working directory by
   any valid Squid build command(s) SHOULD be listed in .gitignore.
   Build commands include ./bootstrap.sh, ./configure, make,
   test-builds.sh, and source-maintenance.sh (each with any supported
   parameters).

2. Untracked directory- or project-specific configuration files for
   development tools commonly used for Squid development MAY be listed
   in .gitignore, provided their inclusion is sponsored by a frequent
   contributor familiar with the tool.

3. Other files SHOULD NOT be listed in .gitignore.
AFAICT, "foo" and "**/foo" mean the same thing.
The sort command in the comment can be copy-pasted to reorder the
already modified file.
The added files were collected (over the course of a few years) by
bootstrapping, configuring, building, and checking Squid sources. Some
entries are likely to be stale, but I am not aware of any specific ones.
FWIW, all but five specific entries do exist in my working directory. No
specific entries are currently tracked by git AFAICT.

Also mark entries containing directory names with a trailing slash to
highlight their elevated impact or special role.

The simple sorting one-liner does not work with negative rules so I
replaced it with at TODO. We can and perhaps should add a simple script
that orders .gitignore accordingly. Alternatively, we can remove the
negative rules for two Makefiles and list all untracked Makefiles
individually instead. Only two Makefiles are currently tracked.
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Using the following policy, I have added the entries I think we should have and removed one entry that I think we should not.

  1. Any untracked file that can be generated in a working directory by any valid Squid build command(s) SHOULD be listed in .gitignore. Build commands include ./bootstrap.sh, ./configure, make, test-builds.sh, and source-maintenance.sh (each with any supported parameters).

  2. Untracked directory- or project-specific configuration files for development tools commonly used for Squid development MAY be listed in .gitignore, provided their inclusion is sponsored by a frequent contributor familiar with the tool.

  3. Other files SHOULD NOT be listed in .gitignore.

Please note that item 2 is a compromise -- I do not actually think those files should be officially added, but I am adding that item because you think they should be (and I hope they will not hurt others much).

@rousskov rousskov dismissed their stale review June 8, 2020 18:39

The current version follows a policy and does not have .bzr, addressing both my concerns.

@kinkie
Copy link
Contributor Author

kinkie commented Jun 8, 2020

i think adding the generated binaries is a bit overkill, but that's my only observation on the additions.
If you feel strongly about it, we can proceed, but otherwise I'd remove them before merging

@yadij
Copy link
Contributor

yadij commented Jun 9, 2020

My objection to all auto-generated files being listed remains strong. Developers, distributors, and users all need to be able to see when their checkout has files that will interfere with the build process, or vice versa be altered by next build. An out-of-tree build directory (eg the btlayer-* pattern) is best practice for development - especially when doing iterative work that needs build objects to stay.

@kinkie
Copy link
Contributor Author

kinkie commented Jun 9, 2020

@yadij what is your proposal then? To not have a .gitignore at all? Or to have what in there?

@rousskov
Copy link
Contributor

rousskov commented Jun 9, 2020

Francesco: i think adding the generated binaries is a bit overkill

FWIW, I do not understand the reasoning behind that statement.

Amos: Developers, distributors, and users all need to be able to see when their checkout has files that will interfere with the build process, or vice versa be altered by next build.

And they are able to see that equally well, with or without the generated files being in .gitignore. Naturally, their setups and commands will depend, in part, on .gitignore.

Each developer can devise an approach that works best for them. This is the question of the best default approach rather than the question of enabling or disabling some fundamental "ability".

@rousskov
Copy link
Contributor

rousskov commented Jun 9, 2020

Francesco: If you feel strongly about it, we can proceed

To avoid misunderstanding, I "feel strongly" about developing and following a good .gitignore membership policy. I hope my proposal, based on a basic "keep git status clean to see repository changes" principle, sets an example. I am ready to support (or abstain from blocking) other good policies (and their matching .gitignores).

FWIW, a bad .gitignore, especially the one created without good membership rules, is a short- and long-term inconvenience/overhead (at least for me). That is the primary reason I am still wasting time on this sad repetition of the #29 fiasco.

@rousskov rousskov self-requested a review June 9, 2020 15:11
@yadij
Copy link
Contributor

yadij commented Jun 11, 2020

@yadij what is your proposal then? To not have a .gitignore at all? Or to have what in there?

Not having .gitignore at all has proven better so far than ad-hoc or a controversial policy.

@rousskov
Copy link
Contributor

rousskov commented Jun 11, 2020

Not having .gitignore at all has proven better so far than ad-hoc or a controversial policy.

For me, having no official .gitignore is better than having a bad official .gitignore. A bad .gitignore is just one more annoyance to work around. I have been there with bzr.

I speculate that for most new Squid developers, having an official .gitignore based on my simple policy would be better than having no official .gitignore because, with that .gitignore, they will see a clean git status -sb that would immediately expose uncommitted/forgotten sources, saving a lot of time and embarrassment. I am sure there are exceptions though, so I do not have much hope in convincing Amos that it is a good policy -- there will always be some poorly specified esoteric use case that I would not be able to exclude as clearly invalid. We have been down that path in #29 .

@kinkie
Copy link
Contributor Author

kinkie commented Jun 21, 2020

I'm closing this PR without merge; it's quite obvious we won't reach any sort of consensus. Sorry for stirring this hornet's nest

@kinkie kinkie closed this Jun 21, 2020
@rousskov rousskov removed their request for review May 20, 2021 03:51
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.

3 participants

Comments