geodesicsolutions-community / geocore-community

GeoCore Community, open source classifieds and auctions software
MIT License
9 stars 6 forks source link

Add .gitattributes to set project specific settings for line feed #119

Closed jonyo closed 2 years ago

jonyo commented 2 years ago

Add a new file to the base, .gitattributes and use it to set behavior for line endings, so it does not convert to CRLF. It should just use LF only. When CR gets injected it seems to break the bash scripts on windows, maybe because they are initiated inside a docker container.

jonyo commented 2 years ago

@vicos59 Don't do it yet, but once this issue is resolved, pull the latest...

Then run this to reset the files:

git rm --cached -r .
git reset --hard

Per the instructions I found here. I'm not done yet but wanted to be sure I did not forget to mention that.

jonyo commented 2 years ago

Hmm, side note: I realized, I think the exclusion of anything that is not *.php in the phpcs ruleset was flawed - when I lifted that part, it started finding a bunch in files like Thing.class.php - I think if there was more than one . it was also excluded.

I couldn't think of a reason not to apply to "any" file, so I'm adjusting the settings and cleaning up the files as part of this one.

jonyo commented 2 years ago

@vicos59 It is now merged into main.

This changes a ton of files, and also a config that only affects when you first check out the repo. So to have the settings take effect @vicos59 like I mentioned above: pull in the latest changes from main. Then make sure you do not have any un-committed changes (run composer status to see). If not then run these 2 commands:

git rm --cached -r .
git reset --hard

After that, bash scripts should run smoothly going forward.

Also, maybe try running cs-fix again and let me know if it ends up changing files for you. If so, start a new issue to look into it, since running cs-fix should have same results when run in docker, so I'll try to look into it further if you still see changes. If you do I'll try loading it up on my game box running windows and see if I can replicate it there.

vicos59 commented 2 years ago

composer status is not recognized by Powershell, so I launched a shell from Docker for geocore-community-web-1 and I am running it from there. Actually, I changed to bash once in the Docker sh.

After running: git rm --cached -r .

It gave me a long list of files---frankly, it looks like everything in src/

for example:

rm 'src/upgrade/versions/pre_2.0.10/index.php' rm 'src/upgrade/versions/pre_2.0.10/progressBar.gif' rm 'src/upgrade/versions/pre_2.0.10/progressBarIE.gif' rm 'src/upgrade/versions/pre_2.0.10/requirement_test.php' rm 'src/upgrade/versions/pre_2.0.10/sql/caeToLatest.sql' rm 'src/upgrade/versions/pre_2.0.10/sql/convertStructure.sql' rm 'src/upgrade/versions/pre_2.0.10/sql/currencyChoices.sql' rm 'src/upgrade/versions/pre_2.0.10/sql/prepareForNew.sql' rm 'src/upgrade/versions/pre_2.0.10/sql/template_changes.sql' rm 'src/upgrade/versions/versions.php' rm 'src/user_images/.empty' root@0fc787413e11:/var/www/html#

But, it looks like the files are still there. Github Desktop now reports 5010 changed files.

git reset --hard said it updated 5010 of 5010 files.

root@0fc787413e11:/var/www/html# git reset --hard
Updating files: 100% (5010/5010), done.
HEAD is now at 4030593 Merge remote-tracking branch 'upstream/main'
root@0fc787413e11:/var/www/html#

And now Github Desktop acts like nothing ever happened. yay!

vicos59 commented 2 years ago

composer cs-fix

root@0fc787413e11:/var/www/html# composer cs-fix
Xdebug: [Step Debug] Time-out connecting to debugging client, waited: 200 ms. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
> php -d memory_limit=2G src/vendor/bin/phpcbf --standard=./contrib/phpcs-ruleset.xml
Xdebug: [Step Debug] Time-out connecting to debugging client, waited: 200 ms. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
/var/www/html/src/config.php                          2      0
/var/www/html/src/geo_templates/t_sets.php            5      0
/var/www/html/src/_geocache/_cacheSettings.php        3      0
----------------------------------------------------------------------
A TOTAL OF 10 ERRORS WERE FIXED IN 3 FILES
----------------------------------------------------------------------

Time: 1 mins, 42.89 secs; Memory: 170.01MB

Script php -d memory_limit=2G src/vendor/bin/phpcbf --standard=./contrib/phpcs-ruleset.xml handling the cs-fix event returned with error code 1
root@0fc787413e11:/var/www/html#

Much better than the last time I ran it.

Github Desktop is not showing any changed files.

I am moving on to try and remove the License page from admin...

jonyo commented 2 years ago

Github Desktop is not showing any changed files.

Great, that's the goal! 🎉

jonyo commented 2 years ago

So after yet another file showed up as having every line change, I realized the job was not complete. After looking into it even more, the issue is that the .gitattributes affects commits going forward. And the line at the top tells git to use lf line endings on any committed text files.

The thing is, there are a lot of files in the project with the crlf endings still. They are not immediately affected by the change, and were not affected by those 2 lines I found it seems. What does affect them is the next time someone tries to change one of those files, on commit it will convert them, so it shows all lines as changed.

I found this write up that really goes in-depth about it, even the history of line endings. In there it gives the magic solution:

git add --renormalize .

Running that it ends up with a ton of files. I think this may be all the remaining hold-outs with the crlf line endings! So it will be a single commit to fix them all, no more being mixed in with other stuff. I wish I had caught this at the start when I was cleaning up the code base so it is part of the initial commit, but oh well. There may be a way to redo the history but I'm not going to bother at this point.