Register repos for auto update.

[gabyx]

No changes had to be made to any tests. They all work. This is the PR for the monologue in #55.

We improved the following:

• All repos which use Githooks hooks are now registered for autoupdates (if not single install) in ~/.gihooks/autoupdate/registered in all circumstances, that means
  • git init -> first use of any hook -> registers this repo
  • git hooks install in repo -> register directly over install.sh
  • git hooks update in repo -> dito because of install.sh
  • running any Githook hook -> registering if not done (cheap)
  • running manually install.sh -> registering directly for any install happening...

An install triggered by a running hook now calls DO_ONLY_UPDATE="yes" install.sh meaning

• if called in a single install repo, we update ony this repo.
• in any other case, we read the ~/.gihooks/autoupdate/registered and cycle through each entry and install the update in each registered repo. In that simple way, every repo receives the new hooks. Add: PR #56 makes this more robust too.

I would be happy if we can get these changes in, also because it makes the whole logic better understandable. (There were some conceptual double usings for example with --single and an update happens.)

Thx for the review, where I also will write some tests afterwards.

[coveralls]

Pull Request Test Coverage Report for Build 505

• 111 of 152 (73.03%) changed or added relevant lines in 4 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.9%) to 86.873%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
base-template.sh	16	20	80.0%
uninstall.sh	53	67	79.1%
install.sh	41	64	64.06%
<!--	Total:	111	152	73.03%	-->

Files with Coverage Reduction	New Missed Lines	%
install.sh	1	86.98%
<!--	Total:	1		-->

Totals
Change from base Build 495:	-0.9%
Covered Lines:	1939
Relevant Lines:	2232

---

💛 - Coveralls

[gabyx]

Alright, I worked hard to make a test (:sleepy:).

[rycus86]

OK, added a few comments, but in general, this looks awesome! Thanks a lot!

[rycus86]

Sorry, commented on the wrong PR. :man_facepalming:

[rycus86]

Added some comments after another review. :+1: This also needs rebasing on master. Thanks!

[gabyx]

Rebased to master and included your review + better comments etc.

[gabyx]

Could you give it another review. I will check the tests tonight.

[rycus86]

Hey! Thanks, I'll try to review when I'm at a computer later. From a quick glance, it looks like there are lots of unresolved comments from last time, could you double check them and reply with your thoughts on them? Thanks a lot! (and happy new year 🎉)

[gabyx]

I rebased and revised all old review marks (some did not even apply anymore). Furthermore I squashed into one commit.

[gabyx]

Test should now work.

[rycus86]

Jad a look over this, looks all fine to me, thanks a lot! I'll do a final pass when I'm at my computer then it should be good to go in. I have some minor nits, but I can fix them up later on master if they still bother me at that point. :)

Thanks again for your hard work on this!

[gabyx]

Thx. and happy new year :-)

[rycus86]

These all look good to me and I almost merged this PR, but then I noticed the new test is actually failing on Travis: https://travis-ci.org/rycus86/githooks/pull_requests

Can you please work out why that is? Thanks!

[gabyx]

Sure, sorry, I thought I fixed it already, probably a typo. Fix in 2-3hours...

[gabyx]

Ok the mistake was a </dev/tty in uninstall.sh. Actually do you know why we actually use /dev/tty instead of stdin in install.sh. I have not yet got that part...

[gabyx]

Almost done. Some wrong sed CURRENT_ESCAPED=$(echo "$CURRENT_REPO" | sed "s@/@\\\\\/@g") instead of CURRENT_ESCAPED=$(echo "$CURRENT_REPO" | sed "s@/@\\\/@g")

[gabyx]

And forgot to skip last update test in 116 if curl/wget is not there

if ! curl --version && ! wget --version; then
    # We skip the update for neither curl or wget
    exit 0
fi

[gabyx]

Now they all shall pass... :-P

[gabyx]

Do you understand why all tests work except the coverage with test116 (my added one?) I think its due to the updat mocking in 116 at the end, this part does not pass in coverage.sh? Why?

[rycus86]

Ah, that one is a special little snowflake... To get coverage on it, we need the otherwise POSIX scripts to run with Bash, and there are some limitations of kcov to work around. Have a look at https://github.com/rycus86/githooks/blob/master/tests/coverage.sh for the things we replace there. My best guess would be perhaps the s/sh/bash/ rewrite doesn't like the DO_UPDATE=yes sh ... or something, but it's just a guess.

I'd appreciate it if you'd try to fix it (you can run it locally with sh tests/coverage.sh or just that one step with sh tests/coverage step-116 I think), but if you can't work it out, I'll merge the PR as it look OK otherwise, then will try to work out the coverage issue if I'll ever have time.

Thanks!

[gabyx]

This regex needed to also match /tmp/test109.1 thats why I added the „.“

I will change to 116.1 of course :). The coverage. I ll have a look tomorrow.

Von meinem iPhone gesendet

Am 23.01.2020 um 21:37 schrieb Viktor Adam notifications@github.com:

@rycus86 commented on this pull request.

In tests/test-whitespace.sh:

@@ -9,7 +9,7 @@ EOF

export ADDITIONAL_INSTALL_STEPS='

add a space in paths and wrap in double-quotes

-RUN find /var/lib/tests -name ".sh" -exec sed -i -E "s|/tmp/test([0-9]+)|\"/tmp/test \1\"|g" {} \; +RUN find /var/lib/tests -name ".sh" -exec sed -i -E "s|/tmp/test([0-9.]+)|\"/tmp/test \1\"|g" {} \; This shouldn't be necessary, /tmp/test109 .1/something should be a valid path. On that note, why 109 and not 116 ? :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

[gabyx]

• I could not find any wrong replaced stuff in all scripts in the container of coverage.sh with

  r=$(pwd)
  docker run -v "$r:/repo" -it githooks:coverage bash -c "cp -r /var/lib/githooks /repo"

• I tested the test 116 with bash in test-alpine-curl it seems to work.

Hm...

[gabyx]

So far, I could not really find out what the problem is. It seems that kcov, seems to hang on the command

echo 'Y
/tmp
' | sh /var/lib/githooks/install.sh || exit 1

only

sh /var/lib/githooks/install.sh || exit 1

Works, do you know why?

Can we merge this, and look for the cause later?

[rycus86]

Yep, thanks for this! Will try to work out what's wrong with the coverage. It's a bit fiddly, I usually just run coverage on a single test, then from the covered lines I can see how far the execution gets, then make a guess what's wrong with the next line. 🤷‍♂️

[rycus86]

@gabyx I just updated and got a bunch of messages, saying:

fatal: --local can only be used inside a git repository

Have you seen this by any chance? I suspect it might be related to this change.

[gabyx]

Ah jep, wuii, this is related to my changes: When we search for repos, I found also cd /home/gabyx/Desktop/Repository/BUILD/GRSFramework/CMakeFiles/git-data/HEAD which is not a git repo. I think we should safe guard after the find with a check if we are inside a git repo if not, we exclude it. I make a fix.

[gabyx]

May be something like that:

    REPOSITORY_LIST=$(
        find "$START_DIR" \( -type d -and -name .git \) -or \
            \( -type f -and -name HEAD -and -not -path "*/.git/*" \) 2>/dev/null
    )

    IFS="$IFS_NEWLINE"
    for EXISTING in $REPOSITORY_LIST; do
        unset IFS

        if [ -f "$EXISTING" ]; then
            # Strip HEAD file
            EXISTING=$(dirname "$EXISTING")
        fi

        if is_git_repo "$EXISTING"; then
            uninstall_hooks_from_repo "$EXISTING"
            EXISTING_REPOSITORY_LIST=$(printf '%s/n%s' "$EXISTING" "$EXISTING_REPOSITORY_LIST")
        fi

        IFS="$IFS_NEWLINE"
    done
    unset IFS

[gabyx]

See #76

[rycus86]

@gabyx not sure if it's related to this PR, but I've noticed with the last 2 updates on my work machine that the installer creates hooks folders in the repositories it was run in, at their root folder, not under their .git subfolder as I'd expect. Can you check if the same happens to you too? When prompted to update, I just say yes search my main dev area, then say yes to install it into previously registered repos too, but skip creating README files.

Thanks!

[gabyx]

I think this is the artefact I tried to correct in #77. For me this does not happen anymore on @master. Maybe delete the autoupdate.register file? Does it happen with a new install? I mean the tests all work, how could that be? Strange.

[rycus86]

Ah yes, that could be it. Maybe there's a way to automatically discard those invalid entries?

[gabyx]

I though I already do that in https://github.com/rycus86/githooks/blob/dd8d92cf7398d855aba6aad3b05d5523f448f905/install.sh#L4301 Aha, but I dont check if its really a git-dir. I only check if its a git repo. I make a PR

[gabyx]

See #79

[gabyx]

😰 ;-)