{"thread":{"id":"49578","subject":"problem with not being able to enforce git content filters","startedAt":"2018-10-16T20:36:42Z","lastAt":"2018-10-17T01:26:55Z","messageCount":7,"participants":["Stas Bekman","Ævar Arnfjörð Bjarmason","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"360664","messageId":"5d4966c8-8171-9a0a-5120-f7c151c2b81f@stason.org","threadId":"49578","inReplyTo":null,"subject":"problem with not being able to enforce git content filters","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-10-16T20:36:37Z","receivedAt":"2018-10-16T20:36:42Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"Hi,\n\nTL;DR\n\nOur open source project dev team has a continuous problem with git\ncontent filters, because developers don't always have them configured.\n\nWe need a way for git to support content filters w/o using user's\n.gitconfig. Otherwise it leads to an inconsistent behavior and messed up\ngit checkouts.\n\n=================================================\n\nFull story:\n\nWe use a version of the nbstripout content filter\n(https://github.com/kynan/nbstripout), which removes user-specific\ninformation from the jupyter notebooks during commit. But the problem\nwould be the same with any one way clean filter.\n\nFirst the setup:\nhttps://github.com/kynan/nbstripout#manual-filter-installation\n\n=================================================\nSet up a git filter using nbstripout as follows:\n\ngit config filter.nbstripout.clean '/path/to/nbstripout'\ngit config filter.nbstripout.smudge cat\ngit config filter.nbstripout.required true\n\nCreate a file .gitattributes or .git/info/attributes with:\n\n*.ipynb filter=nbstripout\n\nApply the filter for git diff of *.ipynb files:\n\ngit config diff.ipynb.textconv '/path/to/nbstripout -t'\n\nIn file .gitattributes or .git/info/attributes add:\n\n*.ipynb diff=ipynb\n\n=================================================\n\nThe problem is that it can't be enforced.\n\nWhen it's not enforced, we end up with some devs using it and others\ndon't, or more often is the same dev sometimes doesn't have it configured.\n\nWhen a person has a stripped out notebook checked out, when another\nperson commits un-stripped out notebook, it leads to: invalid `git\nstatus` reports, `git pull` breaks, `git stash` doesn't work, since it\ntries to stash using the filters, and `git pull' can never succeed\nbecause it thinks that it'll overwrite the local changes, but `git diff`\nreturns no changes.\n\nSo the only solution when this happens is to disable the filters, clean\nup the mess, re-enable the filters. Many people just make a new clone -\nouch!\n\nAnd the biggest problem is that it affects all users who may have the\nfilters enabled, e.g. because they worked on a PR, and not just devs -\ni.e. the repercussions are much bigger than just a few devs affected.\n\nWe can't use server-side hooks to enforce this because the project is on\ngithub.\n\nAnd the devs honestly try to do their best to remember to configure the\nfilters, but for some reason they disappear for them, don't ask me why,\nI don't know. This is an open source project team, not a work place.\n\nYou can see some related complaints here:\nhttps://github.com/kynan/nbstripout/issues/65#issuecomment-430346894\nhttps://stackoverflow.com/questions/51883227/git-pull-stash-conflicts-with-a-git-filter\nand I can find you a whole bunch more if you need more evidence.\n\n==================================================\n\nProposed solution:\n\nThere needs to be a way for a project to define content filters w/o\ngoing via user's .gitconfig configuration, since due to git's security\nit can't be distributed to all users and must be enabled manually by\neach user, which is just not the right solution in this case.\n\nOf course, I'm open to other suggestions that may help this issue.\n\n\"Tell your developers they must configure the filters\" is not it - I\ntried it for a long time and in vain. If you look at our install\ninstructions: https://github.com/fastai/fastai#developer-install\n\n  git clone https://github.com/fastai/fastai\n  cd fastai\n  tools/run-after-git-clone\n\nIt already includes an instruction to run a script which enables the\nfilters, but this doesn't seem to help (and no, it's not a problem with\nthe script). The devs report that the configuration is there for a\nwhile, and then suddenly it is not there, I don't know why. Perhaps they\nmake a new clone and forget to re-enable the filters, perhaps they\ndisable them to clean up and forget to reenable them, I can't tell.\n\nThe bottom line it sucks and I hope that you can help with offering a\nprogrammatic solution, rather than recommending creating a police\ndepartment.\n\nThank you for listening.\n\n-- \n________________________________________________\nStas Bekman       <'))))><       <'))))><\nhttps://stasosphere.com  https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"360671","messageId":"87y3axd21q.fsf@evledraar.gmail.com","threadId":"49578","inReplyTo":"5d4966c8-8171-9a0a-5120-f7c151c2b81f@stason.org","subject":"Re: problem with not being able to enforce git content filters","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-16T21:17:21Z","receivedAt":"2018-10-16T21:18:15Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 16 2018, Stas Bekman wrote:\n\n> When a person has a stripped out notebook checked out, when another\n> person commits un-stripped out notebook, it leads to: invalid `git\n> status` reports, `git pull` breaks, `git stash` doesn't work, since it\n> tries to stash using the filters, and `git pull' can never succeed\n> because it thinks that it'll overwrite the local changes, but `git diff`\n> returns no changes.\n\nPlanting a flag here. Let's get to this later.\n\n> So the only solution when this happens is to disable the filters, clean\n> up the mess, re-enable the filters. Many people just make a new clone -\n> ouch!\n>\n> And the biggest problem is that it affects all users who may have the\n> filters enabled, e.g. because they worked on a PR, and not just devs -\n> i.e. the repercussions are much bigger than just a few devs affected.\n>\n> We can't use server-side hooks to enforce this because the project is on\n> github.\n\nUltimately git's a distributed system and we won't ever be able to\nenforce that users in their local copies use filters, and they might\nedit stuff e.g. in the GitHub UI directly, or with some other git\nimplementation.\n\nSo if you have such special needs maybe consider hosting your own setup\nwhere you can have pre-receive hooks, or within GitHub e.g. enforce that\nall things must go through merge requests, and have some robot that\nchecks that the content to be merged has gone through filters before\nbeing approved.\n\n> And the devs honestly try to do their best to remember to configure the\n> filters, but for some reason they disappear for them, don't ask me why,\n> I don't know. This is an open source project team, not a work place.\n\n*Picks up flag*. For the purposes of us trying to understand this report\nit would be really useful to boil what's happening down to some\nstep-by-step reproducible recipe.\n\nI.e. with some dummy filter configured & not configured how does \"git\npull\" end up breaking, and in this case you're alluding to git simply\nforgetting config, how would that happen?\n\n> There needs to be a way for a project to define content filters w/o\n> going via user's .gitconfig configuration, since due to git's security\n> it can't be distributed to all users and must be enabled manually by\n> each user, which is just not the right solution in this case.\n>\n> Of course, I'm open to other suggestions that may help this issue.\n>\n> \"Tell your developers they must configure the filters\" is not it - I\n> tried it for a long time and in vain. If you look at our install\n> instructions: https://github.com/fastai/fastai#developer-install\n>\n>   git clone https://github.com/fastai/fastai\n>   cd fastai\n>   tools/run-after-git-clone\n>\n> It already includes an instruction to run a script which enables the\n> filters, but this doesn't seem to help (and no, it's not a problem with\n> the script). The devs report that the configuration is there for a\n> while, and then suddenly it is not there, I don't know why. Perhaps they\n> make a new clone and forget to re-enable the filters, perhaps they\n> disable them to clean up and forget to reenable them, I can't tell.\n\nFWIW I tried following these on my Debian install and both on python2\nand python3 I get either syntax errors or a combo of that and a missing\npathlib from that script. I don't know Python well enough to debug\nit. Maybe that's part of the issue?\n\n> The bottom line it sucks and I hope that you can help with offering a\n> programmatic solution, rather than recommending creating a police\n> department.\n\nI think it would be great to have .gitconfig in-repo support, but a lot\nof security & UI problems need to be surmounted before that would\nhappen, here's some previous ramblings of mine on that issue:\nhttps://public-inbox.org/git/?q=87zi6eakkt.fsf%40evledraar.gmail.com\n\nIt now occurs to me that a rather minimal proof-of-concept version of\nthat would be:\n\n 1. On repository clone, detect if HEAD:.gitconfig exists\n\n 2. If it does, and we trust $(git config -f <untrusted file> -l)\n    enough, emit some advice() output saying the project suggests\n    setting config so-and-so, and that you could run the following one\n    liner(s) to set it if you agree.\n\n 3. Once we have that we could eventually nudge our way towards\n    something like what I suggested in the linked threads above,\n    i.e. consuming some subset of that config directly from the repo's\n    HEAD:.gitconfig\n"},{"id":"360676","messageId":"a6dcfee6-595b-2375-e9cb-71da74ed80bb@stason.org","threadId":"49578","inReplyTo":"87y3axd21q.fsf@evledraar.gmail.com","subject":"Re: problem with not being able to enforce git content filters","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-10-16T22:12:58Z","receivedAt":"2018-10-16T22:13:03Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-10-16 02:17 PM, Ævar Arnfjörð Bjarmason wrote:\n[...]\n>> We can't use server-side hooks to enforce this because the project is on\n>> github.\n> \n> Ultimately git's a distributed system and we won't ever be able to\n> enforce that users in their local copies use filters, and they might\n> edit stuff e.g. in the GitHub UI directly, or with some other git\n> implementation.\n\nIn this particular case github won't be a problem, since for the problem\nto appear it has to be executed on the user side. Editing directly in\ngithub UI is not a problem.\n\nJust to give a little big more context to the issue in first place. A\njupyter notebook is a json file that contains the source code, the\noutputs from executing that code and the unique user environment bits.\nWe want to \"git\" the source code but not the rest. Otherwise merging is\na hell. Hence the stripping.\n\nThere are other approaches to solve this problem, besides stripping, but\n they all require some kind of pre-processing before committing the file.\n\n> So if you have such special needs maybe consider hosting your own setup\n> where you can have pre-receive hooks, or within GitHub e.g. enforce that\n> all things must go through merge requests, and have some robot that\n> checks that the content to be merged has gone through filters before\n> being approved.\n\nYes, that would be ideal. But I doubt this is going to happen, I'm just\na contributing developer, not the creator/owner of the project. And as I\nsaid this affects anybody who collaborates on jupyter notebooks, not\njust in our project. I think there are several millions of them on github.\n\n> *Picks up flag*. For the purposes of us trying to understand this report\n> it would be really useful to boil what's happening down to some\n> step-by-step reproducible recipe.\n> \n> I.e. with some dummy filter configured & not configured how does \"git\n> pull\" end up breaking, and in this case you're alluding to git simply\n> forgetting config, how would that happen?\n\nThe problem of 'git pull' and 'git status' and 'git stash' is presented\nin details here:\nhttps://stackoverflow.com/questions/51883227/git-pull-stash-conflicts-with-a-git-filter\nand more here:\nhttps://github.com/kynan/nbstripout/issues/65\nhttps://github.com/jupyter/nbdime/issues/410#issuecomment-412999758\n\nThe problem of configuration disappearing I sadly have no input. It\ndoesn't happen to me, and all I get from others is that it first works,\nand then it doesn't. Perhaps it has something to do with some of them\nusing windows. I don't know, sorry.\n\n>> The bottom line it sucks and I hope that you can help with offering a\n>> programmatic solution, rather than recommending creating a police\n>> department.\n> \n> I think it would be great to have .gitconfig in-repo support, but a lot\n> of security & UI problems need to be surmounted before that would\n> happen, here's some previous ramblings of mine on that issue:\n> https://public-inbox.org/git/?q=87zi6eakkt.fsf%40evledraar.gmail.com\n> \n> It now occurs to me that a rather minimal proof-of-concept version of\n> that would be:\n> \n>  1. On repository clone, detect if HEAD:.gitconfig exists\n> \n>  2. If it does, and we trust $(git config -f <untrusted file> -l)\n>     enough, emit some advice() output saying the project suggests\n>     setting config so-and-so, and that you could run the following one\n>     liner(s) to set it if you agree.\n> \n>  3. Once we have that we could eventually nudge our way towards\n>     something like what I suggested in the linked threads above,\n>     i.e. consuming some subset of that config directly from the repo's\n>     HEAD:.gitconfig\n\nI like it, Ævar!\n\nI feel doing the check and prompting the user on the first push/commit\nafter clone would be a better. It'd be too easy for the user to skip\nthat step on git clone. In our particular case we want it where the\nproblem is introduced, which is on commit, and not on clone. I hope it\nmakes sense.\n\n\n-- \n________________________________________________\nStas Bekman       <'))))><       <'))))><\nhttps://stasosphere.com  https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"360696","messageId":"20181016232636.GF432229@genre.crustytoothpaste.net","threadId":"49578","inReplyTo":"5d4966c8-8171-9a0a-5120-f7c151c2b81f@stason.org","subject":"Re: problem with not being able to enforce git content filters","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-16T23:26:37Z","receivedAt":"2018-10-16T23:26:45Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 01:36:37PM -0700, Stas Bekman wrote:\n> The problem is that it can't be enforced.\n> \n> When it's not enforced, we end up with some devs using it and others\n> don't, or more often is the same dev sometimes doesn't have it configured.\n> \n> When a person has a stripped out notebook checked out, when another\n> person commits un-stripped out notebook, it leads to: invalid `git\n> status` reports, `git pull` breaks, `git stash` doesn't work, since it\n> tries to stash using the filters, and `git pull' can never succeed\n> because it thinks that it'll overwrite the local changes, but `git diff`\n> returns no changes.\n> \n> So the only solution when this happens is to disable the filters, clean\n> up the mess, re-enable the filters. Many people just make a new clone -\n> ouch!\n> \n> And the biggest problem is that it affects all users who may have the\n> filters enabled, e.g. because they worked on a PR, and not just devs -\n> i.e. the repercussions are much bigger than just a few devs affected.\n> \n> We can't use server-side hooks to enforce this because the project is on\n> github.\n> \n> And the devs honestly try to do their best to remember to configure the\n> filters, but for some reason they disappear for them, don't ask me why,\n> I don't know. This is an open source project team, not a work place.\n\nThis sounds like it could be easily solved by continuous integration.\nYou could set up a job on any of a variety of services that checks that\na pull request or other commit is clean when when the filter runs.  If\nit doesn't pass, the code doesn't merge.\n\nThis is what other projects do for style-related and tidiness issues.\nSimilar approaches can be used in other situations to enforce that all\nline endings are LF, or whatever your project desires.\n\nI don't think it's a good idea to provide Git configuration to end\nusers, even with prompting, since there are many novice users who don't\nknow what the security implications of various config options are.  I\nalso personally never would want to be prompted for such a thing, so\neven if that were a feature, people would turn if off, and you'd be no\nbetter off than you were before.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360722","messageId":"038d40b9-0b35-b93f-9377-7c0bb12bce8e@stason.org","threadId":"49578","inReplyTo":"20181016232636.GF432229@genre.crustytoothpaste.net","subject":"Re: problem with not being able to enforce git content filters","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-10-16T23:48:13Z","receivedAt":"2018-10-16T23:48:18Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"\n>> And the devs honestly try to do their best to remember to configure the\n>> filters, but for some reason they disappear for them, don't ask me why,\n>> I don't know. This is an open source project team, not a work place.\n> \n> This sounds like it could be easily solved by continuous integration.\n> You could set up a job on any of a variety of services that checks that\n> a pull request or other commit is clean when when the filter runs.  If\n> it doesn't pass, the code doesn't merge.\n\nThis is an excellent idea wrt to PRs. Thank you, Brian! I will implement\nthat.\n\nIt doesn't help with direct commits to master, since CI would be\ndetecting it after it was committed. And when that happens we all know\nthat already because 'git pull' fails.\n\n\n-- \n________________________________________________\nStas Bekman       <'))))><       <'))))><\nhttps://stasosphere.com  https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"360723","messageId":"20181017005424.GH432229@genre.crustytoothpaste.net","threadId":"49578","inReplyTo":"038d40b9-0b35-b93f-9377-7c0bb12bce8e@stason.org","subject":"Re: problem with not being able to enforce git content filters","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-10-17T00:54:24Z","receivedAt":"2018-10-17T00:54:34Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Oct 16, 2018 at 04:48:13PM -0700, Stas Bekman wrote:\n> \n> >> And the devs honestly try to do their best to remember to configure the\n> >> filters, but for some reason they disappear for them, don't ask me why,\n> >> I don't know. This is an open source project team, not a work place.\n> > \n> > This sounds like it could be easily solved by continuous integration.\n> > You could set up a job on any of a variety of services that checks that\n> > a pull request or other commit is clean when when the filter runs.  If\n> > it doesn't pass, the code doesn't merge.\n> \n> This is an excellent idea wrt to PRs. Thank you, Brian! I will implement\n> that.\n> \n> It doesn't help with direct commits to master, since CI would be\n> detecting it after it was committed. And when that happens we all know\n> that already because 'git pull' fails.\n\nTypically projects that have CI set up don't allow direct pushes to\nmaster, since that tends to allow to bypass CI, or they allow it only in\nexceptional circumstances (e.g., reverts).  Also, typically most\nprojects want to have some sort of review before code (resp. documents,\nother contributions) are merged.  Most Git hosting platforms, including\nGitHub, offer protected branches, so it's something to consider.\n\nThere is a possible alternative, and that's that if your project has\nsome sort of build file (e.g., a Makefile), you can set it up to\nautomatically insert hooks or git configuration into developers'\nsystems, optionally only if it's in a Git repository.  For example, you\ncould add a pre-commit hook that fails if the filters are disabled.\n\nThese are the approaches that tend to work well for most projects.  I\ntend to prefer the CI approach with restricted branches because often I\nwant to customize my hooks, but your project will decide what works best\nfor it.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"360724","messageId":"8660554f-9f48-af67-fbe2-f5aee5e61b09@stason.org","threadId":"49578","inReplyTo":"20181017005424.GH432229@genre.crustytoothpaste.net","subject":"Re: problem with not being able to enforce git content filters","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-10-17T01:26:49Z","receivedAt":"2018-10-17T01:26:55Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-10-16 05:54 PM, brian m. carlson wrote:\n[...]\n>> It doesn't help with direct commits to master, since CI would be\n>> detecting it after it was committed. And when that happens we all know\n>> that already because 'git pull' fails.\n> \n> Typically projects that have CI set up don't allow direct pushes to\n> master, since that tends to allow to bypass CI, or they allow it only in\n> exceptional circumstances (e.g., reverts).  Also, typically most\n> projects want to have some sort of review before code (resp. documents,\n> other contributions) are merged.  Most Git hosting platforms, including\n> GitHub, offer protected branches, so it's something to consider.\n> \n> There is a possible alternative, and that's that if your project has\n> some sort of build file (e.g., a Makefile), you can set it up to\n> automatically insert hooks or git configuration into developers'\n> systems, optionally only if it's in a Git repository.  For example, you\n> could add a pre-commit hook that fails if the filters are disabled.\n> \n> These are the approaches that tend to work well for most projects.  I\n> tend to prefer the CI approach with restricted branches because often I\n> want to customize my hooks, but your project will decide what works best\n> for it.\n\nYes, Brian, what you're sharing makes total sense when things are setup\nthis way, but this is not the case with the project I'm contributing to.\nThis one is setup, commit first, review later, due to having too few hands.\n\nAnd I have already setup a CI to detect misconfigured systems. It'll\ncatch RPs in time, and everything else post-commit. Let's hope the\ndevelopers will watch the status of their commits and will react quickly\nto fix their setup and mend the broken commit, when they see their\ncommit broke things. That's as good as it can get in this particular\nsituation I understand.\n\nThank you, Brian and Ævar for your support and very helpful suggestions.\n\n-- \n________________________________________________\nStas Bekman       <'))))><       <'))))><\nhttps://stasosphere.com  https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"}]}