Skip to content

Warn about orphan .git directories#2359

Merged
jaydoane merged 1 commit intoapache:masterfrom
cloudant:git-dir-warning
Dec 18, 2019
Merged

Warn about orphan .git directories#2359
jaydoane merged 1 commit intoapache:masterfrom
cloudant:git-dir-warning

Conversation

@jaydoane
Copy link
Copy Markdown
Contributor

@jaydoane jaydoane commented Dec 13, 2019

Overview

A .git directory found in src/ usually indicates that the code in the
enclosing directory is managed from said .git directory. This can lead
to confusion, and developers opening PRs against obsolete repos when, as
happens frequently, a formerly separate repository becomes integrated
into the primary repo.

This patch changes the configure script to warn when it finds such .git
directories.

Testing recommendations

Example output:

$ ./configure --dev --skip-deps
==> configuring couchdb in rel/couchdb.config
WARNING unexpected .git directory src/couch_tests/.git
WARNING unexpected .git directory src/rexi/.git
WARNING unexpected .git directory src/mem3/.git
WARNING unexpected .git directory src/smoosh/.git
WARNING unexpected .git directory src/couch_mrview/.git
WARNING unexpected .git directory src/couch/.git
WARNING unexpected .git directory src/couch_replicator/.git
WARNING unexpected .git directory src/ddoc_cache/.git
WARNING unexpected .git directory src/couch_peruser/.git
WARNING unexpected .git directory src/setup/.git
WARNING unexpected .git directory src/couch_log/.git
WARNING unexpected .git directory src/couch_epi/.git
WARNING unexpected .git directory src/mango/.git
WARNING unexpected .git directory src/chttpd/.git
WARNING unexpected .git directory src/couch_stats/.git
WARNING unexpected .git directory src/global_changes/.git
WARNING unexpected .git directory src/couch_event/.git
WARNING unexpected .git directory src/fabric/.git
WARNING unexpected .git directory src/couch_plugins/.git
WARNING unexpected .git directory src/ken/.git
WARNING unexpected .git directory src/couch_index/.git
You have configured Apache CouchDB, time to relax. Relax.

Related Issues or Pull Requests

Checklist

  • Code is written and works correctly
  • Changes are covered by tests
  • Any new configurable parameters are documented in rel/overlay/etc/default.ini
  • A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation

@jaydoane jaydoane force-pushed the git-dir-warning branch 2 times, most recently from d9bd85e to 1955c3e Compare December 17, 2019 17:58
@davisp
Copy link
Copy Markdown
Member

davisp commented Dec 17, 2019

I'm on board with this other than making it conditional. I'd remove the toggle and just run it regardless as it seems like good hygiene for those of us that skip around between major versions fairly often.

Though that also assumes that its not super expensive though it doesn't look like it would be.

@jaydoane
Copy link
Copy Markdown
Contributor Author

@davisp I originally did write it without the conditional, but became concerned about forcing a potentially expensivefind (which I've since improved on by adding -type dir) on everyone each time it was run. Maybe more worrisome though, I'm not sure find is available on Windows, so it seemed best to make it optional, but figured adding it to make dev would probably hit most use cases.

@davisp
Copy link
Copy Markdown
Member

davisp commented Dec 17, 2019

Windows uses the configure.ps1 command thingamajig so that shouldn't be an issue (cc @wohali). For the expensiveness I'm not overly concerned. But if you like then only adding it to --dev seems fine. I don't really see it useful as a standalone option cause I don't think it'd be an issue often enough to really get into muscle memory.

@jaydoane
Copy link
Copy Markdown
Contributor Author

Great, let's make it non-optional since it's simpler.

Copy link
Copy Markdown
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

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

+1 but squash those into a single commit.

A .git directory found in src/ usually indicates that the code in the
enclosing directory is managed from said .git directory. This can lead
to confusion, and developers opening PRs against obsolete repos when, as
happens frequently, a formerly separate repository becomes integrated
into the primary repo.

This patch changes the configure script to warn when it finds such .git
directories.

Example output:

$ ./configure --dev --skip-deps
==> configuring couchdb in rel/couchdb.config
WARNING unexpected .git directory src/couch_tests/.git
WARNING unexpected .git directory src/rexi/.git
WARNING unexpected .git directory src/mem3/.git
WARNING unexpected .git directory src/smoosh/.git
WARNING unexpected .git directory src/couch_mrview/.git
WARNING unexpected .git directory src/couch/.git
WARNING unexpected .git directory src/couch_replicator/.git
WARNING unexpected .git directory src/ddoc_cache/.git
WARNING unexpected .git directory src/couch_peruser/.git
WARNING unexpected .git directory src/setup/.git
WARNING unexpected .git directory src/couch_log/.git
WARNING unexpected .git directory src/couch_epi/.git
WARNING unexpected .git directory src/mango/.git
WARNING unexpected .git directory src/chttpd/.git
WARNING unexpected .git directory src/couch_stats/.git
WARNING unexpected .git directory src/global_changes/.git
WARNING unexpected .git directory src/couch_event/.git
WARNING unexpected .git directory src/fabric/.git
WARNING unexpected .git directory src/couch_plugins/.git
WARNING unexpected .git directory src/ken/.git
WARNING unexpected .git directory src/couch_index/.git
You have configured Apache CouchDB, time to relax. Relax.
@jaydoane jaydoane changed the title Optionally warn about orphan .git directories Warn about orphan .git directories Dec 18, 2019
@jaydoane jaydoane merged commit 26221fa into apache:master Dec 18, 2019
@jaydoane jaydoane deleted the git-dir-warning branch December 18, 2019 07:49
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.

2 participants