| Message ID | CAJ-EccMXEVktpuPS5BwkGqTo++dGcpHAuSUZo7WgJhAzFByz0g@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GIT,PULL] SafeSetID MAINTAINERS file update for v5.3 | expand |
On Wed, Jul 31, 2019 at 2:30 PM Micah Morton <mortonm@chromium.org> wrote: > > You mentioned a couple weeks ago it would be good if I added myself to > the MAINTAINERS file for the SafeSetID LSM. Here's the pull request > for v5.3. There's a lot more than the maintainer ID in there. You've rebased old patches that I already had etc: Jann Horn (10): LSM: SafeSetID: fix pr_warn() to include newline LSM: SafeSetID: fix check for setresuid(new1, new2, new3) LSM: SafeSetID: refactor policy hash table LSM: SafeSetID: refactor safesetid_security_capable() LSM: SafeSetID: refactor policy parsing LSM: SafeSetID: fix userns handling in securityfs LSM: SafeSetID: rewrite userspace API to atomic updates LSM: SafeSetID: add read handler LSM: SafeSetID: verify transitive constrainedness LSM: SafeSetID: fix use of literal -1 in capable hook Micah Morton (2): Merge commit 'v5.3-rc2^0' Add entry in MAINTAINERS file for SafeSetID LSM Not pulled. Linus
Sorry about that. To fix it I did a "git reset hard" to before any of
those commits by Jann Horn, then fast-forwarded to the v5.3-rc2 tag
and force pushed that to my origin/master then pushed a new branch up
with my MAINTAINERS file changes. Hopefully this is a valid fix.
--
The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b:
Linux 5.3-rc2 (2019-07-28 12:47:02 -0700)
are available in the Git repository at:
https://github.com/micah-morton/linux.git
tags/safesetid-maintainers-correction-5.3-rc2
for you to fetch changes up to fc5b34a35458314df1dd00281f6e41f419581aa9:
Add entry in MAINTAINERS file for SafeSetID LSM (2019-08-01 10:30:57 -0700)
----------------------------------------------------------------
Add entry in MAINTAINERS file for SafeSetID LSM.
Has not had any bake time or testing, since its just changes to a text file.
----------------------------------------------------------------
Micah Morton (1):
Add entry in MAINTAINERS file for SafeSetID LSM
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
On Thu, Aug 1, 2019 at 6:25 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Jul 31, 2019 at 2:30 PM Micah Morton <mortonm@chromium.org> wrote:
> >
> > You mentioned a couple weeks ago it would be good if I added myself to
> > the MAINTAINERS file for the SafeSetID LSM. Here's the pull request
> > for v5.3.
>
> There's a lot more than the maintainer ID in there. You've rebased old
> patches that I already had etc:
>
> Jann Horn (10):
> LSM: SafeSetID: fix pr_warn() to include newline
> LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
> LSM: SafeSetID: refactor policy hash table
> LSM: SafeSetID: refactor safesetid_security_capable()
> LSM: SafeSetID: refactor policy parsing
> LSM: SafeSetID: fix userns handling in securityfs
> LSM: SafeSetID: rewrite userspace API to atomic updates
> LSM: SafeSetID: add read handler
> LSM: SafeSetID: verify transitive constrainedness
> LSM: SafeSetID: fix use of literal -1 in capable hook
>
> Micah Morton (2):
> Merge commit 'v5.3-rc2^0'
> Add entry in MAINTAINERS file for SafeSetID LSM
>
> Not pulled.
>
> Linus
On Thu, Aug 1, 2019 at 11:11 AM Micah Morton <mortonm@chromium.org> wrote: > > Add entry in MAINTAINERS file for SafeSetID LSM. So I've pulled this now. However, I have to say that I'm now very nervous about future pulls, simply because the last one had basically everything that can be wrong be wrong. Random rebasing of existing commits, a random merge with no sane merge message.. All complete no-no's. So I will have to remember to be careful when pulling from you, and you need to get into a habit of not doing those things. One very powerful git tool is "gitk". It's just a good idea to use it to *visualize* to yourself what it is you actually have. Do something like git fetch linus (or "upstream" or "origin" or whatever your remote branch for my tree is called) gitk linus/master.. which should show you very clearly what you have that is not in my tree, and should show any odd merges etc. Just doing "git diff" doesn't show garbage _history_, it only shows the differences between the two states. There can be crazy bad history that doesn't show up in the diff, exactly because you had duplicate commits or something like that. Linus
On Thu, Aug 1, 2019 at 11:11 AM Micah Morton <mortonm@chromium.org> wrote: > > The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b: > > Linux 5.3-rc2 (2019-07-28 12:47:02 -0700) > > are available in the Git repository at: > > https://github.com/micah-morton/linux.git > tags/safesetid-maintainers-correction-5.3-rc2 Hmm. This pull request was apparently not caught by pr-tracker-bot for some reason, so it didn't get the automated "this has been pulled" message. I'm not entirely sure why - it was cc'd to lkml, and I see it on lore as https://lore.kernel.org/lkml/CAJ-EccOqmmrf2KPb7Z7NU6bF_4W1XUawLLy=pLekCyFKqusjKQ@mail.gmail.com/ so the email itself made it through the system. And it has "GIT PULL" in the subject line, so the pr-tracker-bot should have looked at it. I see a couple of _potential_ reasons why it might have been overlooked: - maybe the "--" marker after your explanation made pr-tracker-bot go "oh, the rest is just a signature" - the fact that the git link looks more like a regular web thing, and the branch name is on another line. Does pr-tracker-bot only trigger on kernel.org things? - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the subject? but it could be something else too. Adding Konstantin to the participants, since he knows the magic. This is not a big deal, and I have probably missed a lot of other cases where the pr-tracker-bot doesn't react to pull requests, but I really like how it gives a heads-up to people about their pulls without me having to do anything extra, so I generally try to look for failures when I can. Konstantin? Linus
On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote: > - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the > subject? Yes, this is the culprit. Here are the matching regexes: https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41 Normally, pull requests don't come in as replies -- this is the first one that got missed in months, to my knowledge. I can change it to be more permissive -- the only concern would be that we would be scanning more bodies, and there *might* be a chance where we end up tracking the wrong message if someone uses Outlook-style replies (fully quoted messages without '>') and those replies arrive before the original message. Both of these are very unlikely to happen (in fact, I believe using Outlook-style replies on LKML would result in mass outrage). So, just let me know if you'd like me to make this change and I'll modify the regex to be something like '^\S*:?\s*\[GIT' that should match most permutations of Re/Aw/etc. -K
On Mon, Aug 5, 2019 at 7:28 AM Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote: > > On Sun, Aug 04, 2019 at 10:47:54AM -0700, Linus Torvalds wrote: > > - maybe pr-tracker-bot ignores follow-up emails with "Re:" in the > > subject? > > Yes, this is the culprit. Here are the matching regexes: > > https://git.kernel.org/pub/scm/linux/kernel/git/mricon/korg-helpers.git/tree/pr-tracker-bot.py#n41 > > Normally, pull requests don't come in as replies -- this is the first > one that got missed in months, to my knowledge. Most pull requests certainly are proper starts of threads. And I generally wish they were, because I know I myself tend to skim over emails much more quickly if it's some old discussion that I either consider solved or where somebody else is handling it, so if I see a pull request in the middle of a thread, it's much more likely that I'd miss it. It does happen, though. Not just in situations like this, where I replied to the original pull request with some reason for why I wouldn't pull it, and then the fixed pull came in as part of the thread. Al Viro does that to me occasionally, for example. Some discussion about a vfs problem, and then the pull request is in the middle of that thread. You can see an example of that here: https://lore.kernel.org/lkml/20180125002151.GR13338@ZenIV.linux.org.uk/ although in that case it was Davem who merged it (in merge commit 8ec59b44a006. Of course, pr-tracker-bot wouldn't have noticed that one anyway, because it also doesn't have "GIT PULL" or anything like that in the subject line at all. So maybe it's not a great example. I don't know if it's worth changing the pr-tracker-bot rules. I *do* think that the whole unquoted for you to fetch changes up to [hex string] is by far the strongest single signal for a pull request, but it's not clear that it's worth spending a lot of CPU time looking for that unless you have a strong signal in the subject line. So I consider this "solved", and maybe people should just realize that they won't get the automated responses unless they do everything just right. Linus
On Mon, Aug 05, 2019 at 11:20:59AM -0700, Linus Torvalds wrote: >I don't know if it's worth changing the pr-tracker-bot rules. I *do* >think that the whole unquoted > > for you to fetch changes up to [hex string] > >is by far the strongest single signal for a pull request, but it's not >clear that it's worth spending a lot of CPU time looking for that >unless you have a strong signal in the subject line. The way we do it currently is by hooking into public-inbox where the email subject is in the commit log. So for us to grab all new subjects it's a single git call, whereas getting the message body requires a git call per message. This is why we pre-filter by subject, as it's a cheap way to avoid needing to issue hundreds of git calls looking for possible matches in message bodies. >So I consider this "solved", and maybe people should just realize that >they won't get the automated responses unless they do everything just >right. Would you consider recording the message-id of the pull request as part of the commit message? This would be a sure way for us to be able to catch all possible cases. In fact, this would allow me to throw out most of the bot logic, as it would become unnecessary. E.g. the merge commit would look like: Merge tag 'foo' of git://git.kernel.org/bar Pull foo features * foo * bar * baz Link: https://lore.kernel.org/r/<message-id> However, I suspect that getting message-ids for all your pull requests would significantly complicate your workflow. -K
On Mon, Aug 5, 2019 at 12:11 PM Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote: > > However, I suspect that getting message-ids for all your pull requests > would significantly complicate your workflow. Yeah, that would be a noticeable annoyance. If I were to process pull requests the way I used to process emailed patches (ie "git am -s" on a mailbox) that would be a natural thing to perhaps do, but it's not at all how it ends up working. Having to save the pull request email to then process it with some script would turn it into a chore. I think the pr-tracker-bot clearly catches most cases as it is, and it's only the occasional "somebody did something odd" that then misses an automated response. Not a huge deal. For me it was actually more the "I didn't understand why the response didn't happen", not so much "I really want to always see responses". Linus
On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote: >> However, I suspect that getting message-ids for all your pull >> requests >> would significantly complicate your workflow. > >Yeah, that would be a noticeable annoyance. If I were to process pull >requests the way I used to process emailed patches (ie "git am -s" on >a mailbox) that would be a natural thing to perhaps do, but it's not >at all how it ends up working. Having to save the pull request email >to then process it with some script would turn it into a chore. > >I think the pr-tracker-bot clearly catches most cases as it is, and >it's only the occasional "somebody did something odd" that then misses >an automated response. Not a huge deal. For me it was actually more >the "I didn't understand why the response didn't happen", not so much >"I really want to always see responses". Ok, let me add a fix for Re: at the start -- this won't make things significantly more expensive, but will catch this particular corner case. Best regards, -K
On Mon, Aug 5, 2019 at 12:27 PM Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote: > > On Mon, Aug 05, 2019 at 12:17:49PM -0700, Linus Torvalds wrote: > >> However, I suspect that getting message-ids for all your pull > >> requests > >> would significantly complicate your workflow. > > > >Yeah, that would be a noticeable annoyance. If I were to process pull > >requests the way I used to process emailed patches (ie "git am -s" on > >a mailbox) that would be a natural thing to perhaps do, but it's not > >at all how it ends up working. Having to save the pull request email > >to then process it with some script would turn it into a chore. > > > >I think the pr-tracker-bot clearly catches most cases as it is, and > >it's only the occasional "somebody did something odd" that then misses > >an automated response. Not a huge deal. For me it was actually more > >the "I didn't understand why the response didn't happen", not so much > >"I really want to always see responses". > > Ok, let me add a fix for Re: at the start -- this won't make things > significantly more expensive, but will catch this particular corner > case. > > Best regards, > -K Linus, thanks for the tips earlier about gitk. I'll use that in the future. Unfortunately I didn't have the mental model quite right of what happens during the pull request. I was thinking along the lines of my commits being cherry picked onto your tree, rather than how it actually happens with git merge where my tree's commit history needs to match yours perfectly.
On Tue, Aug 6, 2019 at 9:32 AM Micah Morton <mortonm@chromium.org> wrote: > > Unfortunately I didn't have the mental model quite right of what > happens during the pull request. I was thinking along the lines of my > commits being cherry picked onto your tree, rather than how it > actually happens with git merge where my tree's commit history needs > to match yours perfectly. The "cherry-pick" model is what "git pull --rebase" does in reverse (ie it pulls the exact history from the other end, and then rebases the _local_ history on top of that). But the cherry-picking model is entirely inappropriate for any bigger project. Yes, you can do it locally on your _local_ small changes (but see all the docs about why rebasing is not a great thing), but it's entirely unmanageable and doesn't scale in the big picture. It's why all the projects that were based on patch series were complete and utter failures. A "patch series" only works locally. It's not reasonable to scale and distribute. Git fundamentally makes history a first-class immutable citizen, and it's a major feature and a core design thing, and it's the _only_ thing that makes distribution possible. Whenever you rewrite history, you fundamentally screw up a distributed model. Linus
Hi Linus, You mentioned a couple weeks ago it would be good if I added myself to the MAINTAINERS file for the SafeSetID LSM. Here's the pull request for v5.3. Thanks! -- The following changes since commit 179757afbef5f64b9bd25e6161f72fc1a52a8f2e: Merge commit 'v5.3-rc2^0' (2019-07-31 13:45:16 -0700) are available in the Git repository at: https://github.com/micah-morton/linux.git tags/safesetid-maintainers-5.3-rc2 for you to fetch changes up to 7e20e910eabdf0af90fd10e712f15b413be8e135: Add entry in MAINTAINERS file for SafeSetID LSM (2019-07-31 13:58:11 -0700) ---------------------------------------------------------------- Add entry in MAINTAINERS file for SafeSetID LSM. Has not had any bake time or testing, since its just changes to a text file. ---------------------------------------------------------------- Micah Morton (1): Add entry in MAINTAINERS file for SafeSetID LSM MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+)