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

.gitignore recommendation in 'environment Git Information' modal should be refined #291

Closed
caldenjacobs opened this issue Jul 19, 2018 · 13 comments

Comments

@caldenjacobs
Copy link

caldenjacobs commented Jul 19, 2018

The current warning reads something like this:
Aegir files were detected by git. It is recommended to add the following to your .gitignore file: Aegir files sites/sites.php sites/*/files sites/*/private sites/*/drushrc.php sites/*/settings.php sites/*/local.settings.php sites/all/drush/drushrc.php

We should be dynamically adding in the specified web root, e.g. if the project web root has been defined as "docroot", each line should be prefixed with "docroot/":
Aegir files docroot/sites/sites.php docroot/sites/*/files docroot/sites/*/private docroot/sites/*/drushrc.php docroot/sites/*/settings.php docroot/sites/*/local.settings.php docroot/sites/all/drush/drushrc.php

It may make sense to also integrate this warning into the project or environment creation process.

@caldenjacobs
Copy link
Author

caldenjacobs commented Jul 19, 2018

I patched Drupal core once (😬) using Composer-patches, and on devshop deploy, drushrc.php and other configuration files were removed, which took my (development) site offline, and broke devmaster's connection with the site until drushrc.php was manually restored. I hadn't added drushrc.php to my repository's .gitignore file.

There may be a better way to prevent something like this from happening in the future, but regardless I'm sure that improving the .gitignore recommendation's interface language and priority will save some Devshop users from making a related mistake.

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

Thank you for the detailed bug report.

I would like to try and make the git ignore setup automatic, so dynamically outputting it would be a good first step.

I'm thinking the composer patches bug warrents a separate issue. Mind posting another?

@caldenjacobs
Copy link
Author

caldenjacobs commented Jul 19, 2018

Can do, although I don't think it's a separate bug. It's doing what it's intended to do (remove Drupal core in order to re-add it and apply the patch), the only issue is that it's removing (web)/sites/*/drushrc.php because it's not ignored by git. I think automating the git ignore setup solves the problem. Thoughts? I'll definitely create another issue if that'd be helpful.

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

I see, so the compose-patches process reverts the git status essentially?

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

I'm starting to wonder, is there a way to drop a .gitignore file that is NOT included in the repo?

Like, if we were to write a /sites/DOMAIN.com/.gitignore file specifically for aegir-generated files, would that prevent them from appearing in git?? I will test this...

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

nope. Maybe there's another way.

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

According to https://git-scm.com/docs/gitignore

  1. Files to exclude may be put into a .git/info/exclude file.
  2. A global config for the user can be set at $HOME/.config/git/ignore

Aegir could technically write either of these automatically.
I think this could be the best way forward.

thoughts?

@helmo
Copy link
Contributor

helmo commented Jul 19, 2018

Suggesting the user to create such a global exclude file sounds like a nice solution. It's easier then adding it to all repo's.
But it has the disadvantage that it would not automatically be on a developer workstation.

PS: Another 'default' location is ~/.gitignore

@jonpugh
Copy link
Member

jonpugh commented Jul 19, 2018

I'm thinking, we should get specific as possible. excluding "drushrc.php" without specifying the path might have unexpected consequences.

I'm thinking Hosting_git module should be able to write the .git/info/exclude file per repo. Aegir already knows the "repo_path", and we have a list of sites on each platform, so we could very specifically add a line for each file aegir writes to that file

ie...

.git/info/exclude:

web/sites/domain1.com/drushrc.php
web/sites/domain1.com/settings.php
web/sites/domain2.com/drushrc.php
web/sites/domain2.com/settings.php

There could be global drush options to prevent this from happening, and to tweak how it's done, if needed.

@caldenjacobs
Copy link
Author

caldenjacobs commented Jul 19, 2018

I like the idea of a git exclude of each specific site's Aegir config files, taking into account the custom site root directory from the project configuration. This approach would need to be documented so that developers don't overlook this configuration and attempt to make changes to these configuration files using their git repo (although developers should be making any necessary changes in local.settings.php anyhow).

Alternatively perhaps we could just recommend that these lines be committed and pushed to each site repository's .gitignore file like so, on environment create:

[web root directory populated from project settings/]sites/*/drushrc.php
[web root directory populated from project settings/]sites/*/settings.php

@caldenjacobs
Copy link
Author

caldenjacobs commented Jul 23, 2018

Another possible solution when using drupal-composer/drupal-project, may be to list drushrc.php in the drupal-scaffold excludes configuration.

https://github.com/drupal-composer/drupal-scaffold#configuration

Thoughts?

@stale
Copy link

stale bot commented Mar 6, 2020

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

@stale stale bot added the stale label Mar 6, 2020
@stale
Copy link

stale bot commented Apr 5, 2020

This issue has been closed due to inactivity. If you feel this is in error, please reopen the issue or file a new issue with the relevant details.

@stale stale bot closed this as completed Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants