On Mon, Nov 16, 2015 at 12:23 AM, Jacob Keller <jacob.keller@xxxxxxxxx> wrote: > On Sun, Nov 15, 2015 at 2:14 PM, Johan Herland <johan@xxxxxxxxxxx> wrote: >> A related topic that has been discussed (although I cannot remember if >> any conclusion was reached) is whether to allow more notes operations >> - specifically _read-only_ operations - on notes trees outside >> refs/notes/. I believe this should also become possible, although I >> haven't thoroughly examined all implications. > > This was discussed at some point on one of the versions of my patch. > The tricky part is in how to get it implemented correctly. > > We need to be able to correctly handle DWIM logic for things, and > ensure that what we're operating on actually looks "note-like" since > we don't really want to perform read-only ops on refs that don't hold > notes like objects. I believe read-only operations on non-notes trees is harmless (although suboptimal). When reading in a notes tree, the notes code maintains non-note entries in a sorted linked list. Only paths that contain exactly 40 hex characters (modulo '/') ends up as "notes" (i.e. false positives). The rest ends up in the non-notes list. The overwhelming majority of non-notes trees will have no "notes" in them (zero false positives). For those few trees that do contain note-like paths: since we never write out the tree again, we don't end up corrupting the non-notes tree itself (which would typically look like changing the "fanout" of note-like paths, e.g. moving 'de/adbeef...' to 'deadbeef...'). Hence, the only damage we can get from reading in a non-notes tree depend on what we subsequently do with the "notes" information read from that tree. Again, since the number of "notes" read from a non-notes tree is typically zero, the subsequent damage is typically, also, zero. For "git notes merge", false positives from a non-notes tree are merged into the first (proper) notes tree. For "git log --notes", false positives end up being displayed as part of the output. Note that here, a false positive must not only match the above criteria (40 hex chars, modulo '/'), but must also correctly name a commit that occurs in the log. Are there other cases where a false positive would wreak considerable havoc? Additionally, if we suspect that passing non-notes trees to read-only operations will be a common error, we could add a simple heuristic to the notes code, to warn (or even abort) if we strongly suspect that we are reading in a non-notes tree. For example, if the ratio of non-notes to notes entries goes above, say, 1:1 (or even 10:1), then what we're reading is probably not a proper notes tree... ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html