cksum authorization

[rspeed]

Merge conflict is because I branched from the parent of 448ceefd3a19a91ac124328ae722e5b307c9d62d.

[blueyed]

I can't get the unit tests working

What does not work? Have you installed cram? (shouldn't be required - it is fine to fix it with my help/Travis probably) I will review the PR later/tomorrow.

[rspeed]

Yeah, I installed cram through pip, but no matter what I do every test fails with this message:

AUTOENV_AUTH_FILE is not in /tmp. Aborting. (no-eol)

[blueyed]

I've also played around with automatically upgrading old whitelist entries when they're encountered, though it's not currently in place.

I would prefer that, but it might not be worth the effort: there are likely only about a dozen authenticated files typically at most, and it's OK to get "reminded" of them probably?! Thinking forward though, we might want to add the version to the auth file, and then grep for the file, and look at the version and hash from there (no additional external program being called). But still: I'd rather want the hashes being upgraded to the faster one than adding more logic to fallback to old versions - so in this case it would only help with upgrading lazily/on-the-fly.

What about just an extra script that cleans and upgrades the auth file? It could look at each entry, and for missing files remove them (with confirmation?), and for changed auth hashes update them (basically what gets done now on hash mismatch).

[rspeed]

I would prefer that, but it might not be worth the effort

It's actually ridiculously easy. Just call _autoenv_authorize when the v1 hash is found. I just didn't want to clobber the old entries.

I just pushed another change which does exactly that.

[blueyed]

Cool, thanks! I've fixed the existing tests and added new ones for the upgrading, where I've noticed that there is a minor issue with _autoenv_deauthorize in case there is only a single entry in the auth file. Fixed this by using a temporary file with grep -v instead.

[rspeed]

I should probably point out that prior to 664df88f4ba82f69f113aa0f1956703744d9afa2, my fork used a slightly different format for the whitelist entry. I was only including the hash portion of the cksum output, whereas @blueyed's code includes the file size as well. Better to not toss out entropy!

Edit: Though the format will change if this PR is merged, since I incremented the entries' version number. So new recently created entries won't be recognized.

[blueyed]

Yeah, we should merge this soonish. @Tarrasch ?

[blueyed]

@rspeed Would be good if you could rebase this on master, squashing everything into a single commit - you can take over ownership of the tests I've fixed/added, but feel free to add some X-Helped-By etc (but not necessary).

I've tried to rebase it, but there are conflicts (like you mentioned). Might be rather trivial though (I've not looked too closely), so I could also look at it later / tomorrow.

[rspeed]

Is "no" an acceptable answer? I'm very much opposed to rewriting SCM history.

[blueyed]

But it certainly breaks git-bisect.

[rspeed]

I have no opposition to rebasing it over to the master branch, only with squashing commits. Though even then, wouldn't that require that I create a new PR?

[blueyed]

No, you can force-push to this branch/PR.

[rspeed]

I have no idea how to do that.

[Tarrasch]

@rspeed, you could probably google it. :)

Also you don't have permissions to "break" anything. So just try until you make it. ^^

[rspeed]

Yeah, that's exactly the problem. I don't have the necessary permissions to push to the PR directly.

[blueyed]

@rspeed Of course.. it is your branch. Basically git push --force-with-lease should work. (--force-with-lease is a bit safer than -f).

[rspeed]

I can push to my branch all I want, it's still going to be cksum-authorization.

[rspeed]

There are three options:

1. Explain how I can tell github to look at my fork's master branch instead of cksum-authorization.
2. Come to terms with the fact that feature branches are a reality.
3. Just do it yourself because you actually have the proper permissions to pull from my fork.

I was happy to help but this is absurd.

[blueyed]

I can push to my branch all I want, it's still going to be cksum-authorization.

Yes, that will update the PR after all.

1. Explain how I can tell github to look at my fork's master branch instead of cksum-authorization.

Why should it look at master? (that is the point of feature branches after all - you can force-push to them).

I was happy to help but this is absurd.

Sorry to have annoyed you, but it seems like we have some misunderstanding somewhere..

[rspeed]

Your concern seemed to be that git-bisect wouldn't work, which I can only see as being a problem due to the fact that my merge occurred on a branch with a name other than master.

[blueyed]

Your concern seemed to be that git-bisect wouldn't work, which I can only see as being a problem due to the fact that my merge occurred on a branch with a name other than master.

Huh? You lost me there.

My concern is that git-bisect should be good on zsh-autoenv's master. This means that we should not merge commits which would fail by itself, i.e. running the tests on every commit should be good. The point here is that if you run git-bisect for some other bug, it should not fail on commits being fixed in a later commit of some merge.

[Tarrasch]

@blueyed, hey, won't it be enough to just do a "Squash and merge" as is possible from the GitHub WebUI these days?

[blueyed]

@Tarrasch Done.

@rspeed Thanks!

[Tarrasch]

Thanks to both of you for your patches @rspeed and @blueyed. :)