Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
Jeff King p...@peff.net writes: This is highlighting the problem with pager.* that Junio mentioned recently, which is that the keyname has arbitrary data,... Yes, even if it is not arbitrary (imagine we limit ourselves to the official set of commands we know about), the naming rule for the git subcommand names should not be dictated by the naming rule for the configuration variables, as they are unrelated. That is one of the reasons why I had the unbounded set, including the ones under our control such as subcommand names in the draft update for the guideline. I dropped that part after the discussion to keep other obviously agreed parts moving, but we may have to revisit it later. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git for collaboration on RDF data
I'm interested in a collaboration and change management solution for data stored in pre-existing RDF data stores set behind SPARQL endpoints. I would like some input on my idea before I invest too much time in reading about Git internals. My main question is whether people more experienced in how Git works internally think that my problem could be solved by using git itself or if I would be better served by developing my own toolkit. These first four paragraphs are to summarize why I'm even thinking of this solution. I consider that externally managing the versioning of data and not including that information in the data store would greatly reduce the usefulness of tracking changes. For example, if multiple versions are exposed through the SPARQL endpoint, readers would be able to compare versions through querying with SPARQL rather than by referring back to a serialized representation of the data in an adjacent repository. This is most pertinent when the data store is accessed by non-human agents since I expect that modifying a query or two in such an agent is easier than adding a feature for reading from an adjacent repository using a distinct set of protocols. Beyond that are the dangers of expecting a different set of data than you receive and how that's difficult to know without cryptographic guarantees of version information. I like the idea of using Git since it has gained a wide acceptance and general understanding, even among the people outside of the software development profession, who I expect will be generating most of the data to be tracked. Then, when it comes to collaboration, I can see that if, for example, I generate some preliminary data in my lab and I want to share it in RDF, branching like in Git allows me to set off this preliminary data, but make it available to peers while still relating it to previously existing data. My initial requirements for this solution are that commits and merges shouldn't slow in the time it takes to complete them in proportion to the size of the database since I want to track stores that can grow to be millions of statements and several gigabytes in size. Based on my expectations of the size of data being managed, I also think that partial sharing of a repository would be useful, but I'm not certain that this is a requirement. My idea is to embed at least the object graph of Git in the managed RDF graph and to make it possible to clone the tracked portion of a graph by using SPARQL queries. Blobs would correspond to named graphs in RDF and their hashes would be computed from a serialization of the graph with canonicalized BNodes, and trees would be sets of triples linking tree nodes to tree entry nodes to named graph identifiers paired with blob object ids. For actually manipulating the commit graph, I expect either to write my own tools or to use FUSE to expose the RDF graph as a file system that git can manipulate like it does for normal source code repositories. I like the second option, first, because it means people can use readily available tools in a way analogous to how they already use them, and, second, because it allows for accessing features of Git to manipulate the RDF graph (for better or worse) in ways that I don't have to explicitly define. My present concerns for this second option are that I don't know yet everything that git does on a file system to simulate it and whether I would like the RDF graph that solution generates. The advantage of the first option is that I know better what to expect if I go that route, and the disadvantages are, essentially, the advantages of the second. Any comment or criticism is welcome. -- Cheers, Mark W. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
Jeff King p...@peff.net writes: On Fri, Feb 06, 2015 at 01:45:28PM +0100, Andreas Krey wrote: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? ... So it is not a new problem, but it is a bug that you cannot set pager config for such a command or alias. Hmm, I think these are two separate issues. (1) you cannot define alias.my_merge because that is not a valid key. We cannot add a new official subcommand git c_m_d because users cannot define pager.c_m_d for it for the same reason. (2) git no-such-command does not get these extraneous error messages, but git no_such_command does. Solution to (1) would be to move to alias.my_merge.command = ... and pager.c_m_d.enabled = true. But I do not think that would solve (1) until we transition and start ignoring alias.my_merge and pager.c_m_d, and I do not think of a way other than squelching the messages to solve (1) during the transition period. I can think of a few possible paths forward: 1. Squelch the messages, and declare show_ref and friends out-of-luck for pager config or aliases. 2. Relax the syntactic rules for config keys to allow more characters. We cannot make this perfect (e.g., we cannot allow . for reasons of ambiguity), but I imagine we could cover most practical cases. Note that we would need the matching loosening on the file-parsing side. 3. Start phasing in pager.*.enabled (and I guess pager.*.command). We would still do the lookup of pager.* for backwards compatibility, but we would be careful to do so only when it is syntactically valid. IOW, this looks like (1), except the path forward for show_ref is to use the new, more robust, syntax. I guess I ended up reaching the same conclusion; 3. with also alias.*.command as the longer-term goal. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Fri, Feb 06, 2015 at 11:44:38AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Fri, Feb 06, 2015 at 01:45:28PM +0100, Andreas Krey wrote: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? ... So it is not a new problem, but it is a bug that you cannot set pager config for such a command or alias. Hmm, I think these are two separate issues. Yeah, sorry, if I wasn't clear. The error messages are definitely a separate and newer issue, and need to be silenced one way or the other. It is just that they are notifying us of a deeper problem that has existed for a long time, and it probably makes sense to deal with both. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 0/7] cpu: add device_add foo-x86_64-cpu support
On Thu, Feb 05, 2015 at 12:17:15PM -0800, Junio C Hamano wrote:
Would length() 1 be enough[1]? Or are people really typing yes and
not just y?
I cannot imagine a charset name that is smaller than two characters. It
may be that there are none smaller than 4, and we could cut it off
there. Googling around for some lists of common charsets, it seems like
that might be plausible (but not any larger; big5 is 4 characters, and
people may spell utf8 without the hyphen).
-Peff
[1] Of course, to match the existing regex code, we may want to spell
this as /../ or //.
Perhaps. Just in case there were shorter ones, something like this
with confirm_only to allow them to say Yes, I do mean 'xx'?
git-send-email.perl | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 3092ab3..848f176 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -752,6 +752,7 @@ sub file_declares_8bit_cte {
print $f\n;
}
$auto_8bit_encoding = ask(Which 8bit encoding should I declare
[UTF-8]? ,
+ valid_re = qr/.{4}/, confirm_only = 1,
default = UTF-8);
}
Yes, I think leaving an escape hatch is a good idea, just in case.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
Kyle J. McKay mack...@gmail.com writes: Actually I just tested it. If we #undef it we could end up producing these: error: syntax error before DEPRECATED_ATTRIBUTE So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. Doesn't the fact that your test failed indicates that it is not jsut to be safe in case but is required for correctness? The first hit for MAC_OS_X_VERSION_MIN_REQUIRED was this: https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/AvailabilityMacros.h which marks quite a many macros to that value. I do not know what changes they make to openssl/*.h (which is included just after the above header is included, but I would imagine that is where the AVAILABLE_MAC_OS_X_VERSION_XXX_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_VERSION_YYY macros are checked and annoying warnings that are being squelched by the previous change are given? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Feb 6, 2015, at 02:00, Eric Sunshine wrote: On Fri, Feb 6, 2015 at 4:35 AM, Kyle J. McKay mack...@gmail.com wrote: #ifndef NO_OPENSSL +#ifdef __APPLE__ #define __AVAILABILITY_MACROS_USES_AVAILABILITY 0 -#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6 +#include AvailabilityMacros.h +#undef DEPRECATED_ATTRIBUTE +#define DEPRECATED_ATTRIBUTE +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY +#endif #include openssl/ssl.h #include openssl/err.h -#undef MAC_OS_X_VERSION_MIN_REQUIRED -#undef __AVAILABILITY_MACROS_USES_AVAILABILITY DEPRECATED_ATTRIBUTE is a fairly generic name. Do we want to be extra careful and #undef it here to avoid potential unintended interactions with other #includes (Apple or not)? The new patch only mucks with it when we have #ifdef __APPLE__ and pretty much any apple code is going to end up including the availability stuff sooner or later which manipulates DEPRECATED_ATTRIBUTE. Essentially that makes DEPRECATED_ATTRIBUTE a reserved macro name on __APPLE__. #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif If we do that, won't the compiler then helpfully supply a 0 when the macro is used? Or is that only when an undefined macro is used inside an #if or #elif ? That would break all later declarations that use it. On the other hand, we've already mucked with it, so #undef may be superfluous. Actually I just tested it. If we #undef it we could end up producing these: error: syntax error before ‘DEPRECATED_ATTRIBUTE’ So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. The worst that happens is you don't see some deprecation warnings that you otherwise would. It won't make any functions available that shouldn't be or make unavailable functions that should be. -Kyle-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] apply: do not allow reversing a 'copy' patch
On Fri, Feb 6, 2015 at 3:06 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: + die(_(sorry, cannot apply a 'copying' patch in reverse (yet))); Is it wise to give the reader of the output hope that this is not implemented (yet) but may be in the future? I'd drop the sorry as well as the (yet) as the style of this error message seems to differ from other error messages in git. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
'git rebase' silently drops changes?
Hello, I recently ran into an annoying problem: 'git rebase' apparently silently drops changes in non-conflicting paths of merge commits (git version 1.9.3). Is it a bug or feature? Is there a way to flatten history using rebase, yet preserve manual changes found in merge commits? Here is simplified reproduction of what I've encountered: SCRIPT git init t cd t git config rerere.enabled true echo I a; git add a echo I b; git add b git commit -am I git checkout -b test echo B b; git commit -m B -a git checkout master echo A a git commit -am A git merge --no-edit test # Clean merge, but result didn't compile, so I fixed it and # amended the merge: echo Precious! a # [!] This is modification that gets lost git commit --amend --no-edit -a cat a # Now rebase my work. git rebase -f HEAD~1 # What? Where is my Precious change in a??? cat a /SCRIPT I.e., the modification marked [!] was silently lost during rebase! -- Sergey. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] apply: do not allow reversing a 'copy' patch
Junio C Hamano gits...@pobox.com writes:
Action item: warn users against using apply -R on a
patchset with such a patch while this is fixed.
This needs to be replaced with the logic to properly reverse a patch
that creates a new file by copying from somewhere else, but for now,
this would be a good idea to prevent lossages from happening.
builtin/apply.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0aad912..b33b403 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2069,6 +2069,9 @@ static void reverse_patches(struct patch *p)
for (; p; p = p-next) {
struct fragment *frag = p-fragments;
+ if (p-is_copy)
+ die(_(sorry, cannot apply a 'copying' patch in reverse
(yet)));
+
swap(p-new_name, p-old_name);
swap(p-new_mode, p-old_mode);
swap(p-is_new, p-is_delete);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Fri, Feb 6, 2015 at 3:43 PM, Kyle J. McKay mack...@gmail.com wrote: On Feb 6, 2015, at 12:05, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. Doesn't the fact that your test failed indicates that it is not jsut to be safe in case but is required for correctness? [...] I do not know what changes they make to openssl/*.h (which is included just after the above header is included, but I would imagine that is where the AVAILABLE_MAC_OS_X_VERSION_XXX_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_VERSION_YYY macros are checked and annoying warnings that are being squelched by the previous change are given? Yes. Although Eric didn't specify exactly where when he suggested adding this: On Feb 6, 2015, at 02:00, Eric Sunshine wrote: #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif I took the suggestion to be after the openssl/*.h headers are included which would avoid the error of having DEPRECATED_ATTRIBUTE be #undef'd for them. But, even math.h can end up including AvailabilityMacros.h, so I think #undef'ing DEPRECATED_ATTRIBUTE after the openssl/*.h headers are included would be unsafe in general. While we might happen to get away with that today, if say compat/apple-common-crypto.h changes in the future (or for that matter any sequence of includes in other files or any headers in the Apple SDK) we could start seeing the error. TLDR; yeah, DEPRECATED_ATTRIBUTE needs to remain defined to nothing. I agree with this analysis. An alternative would be to revert b195aa00 (and not apply this patch), but then we risk having legitimate Git-related compilation warnings lost in the noise of the useless Apple deprecation warnings. Given the glacial pace at which Apple headers changes, and considering the rapid pace of change in Git, it still seems the lesser evil to suppress the useless warnings Apple thrust upon us when they deprecated OpenSSL in its entirety without providing replacements. It's unfortunate that the DEPRECATED_ATTRIBUTE #define will bleed beyond the OpenSSL #includes and potentially allow us to miss some future Apple deprecations, however, given the shortcomings of b195aa00, the proposed patch seems to be the best we can do. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFH] diff -B -M
Junio C Hamano gits...@pobox.com writes: 5. Third twist: rewriting by copying ... One possible way to fix this is to include another patch in the same patchset that shows the deletion of major-11.txt. The rule 2. would be further revised to something like: rule 2. a patch renaming file A to file B requires that file A exists and file B does not exist in the target tree, unless another patch in the patchset renames file B to some other file (possibly but not necessarily file A) or removed file Typofix: s/removed/removes/ B, in which case file B must appear in the target tree. Such a patchset would look like this: diff --git a/major-08.txt b/major-11.txt similarity index 97% rename from major-08.txt rename to major-11.txt ... diff --git a/major-11.txt b/major-11.txt deleted file mode 100644 ... And these patches, under the re-revised rule 2. and rule 4., would apply cleanly to the old tree. What about the reverse application? It would be a patchset that creates major-11.txt from nothingness, and creates major-08.txt by renaming major-11.txt and editing. Is the rule 2. re-revised above sufficient? renaming major-11.txt to major-08.txt requires that major-08.txt does not exist in the target tree, unless... and the new tree (which is the target of the reverse application) only has major-11.txt and not major-08.txt, so this rename should go through. The reverse of the deletion of major-11.txt is a creation of it with the contents fully given as the pre-image of the (original) patch before reversing it, so that should also be OK with rule 3. ... But rule 3. needs tweaking. The second patch would be creating major-11.txt which requires major-11.txt to be missing in the target tree. But obviously, the new tree the patch we are applying in reverse was taken from has major-11.txt. So we would need a revision to it as well. rule 3. a patch that creates file A requires that file A does not exist in the target tree, unless another patch in the same patchset renames file A away or removes it. So tentatively, I would say... Action item: do not filter delete-half of the broken pair out, even when the other create-half of the pair no longer is in the output. Action item: update git apply to honor the rule 2. re-revised above. The latter should read: update git apply to honor the revised rule 2. and 3. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFH] diff -B -M
I've been thinking about diff -B -M for quite a while, and I am finding a few interesting problems we have in our codebase. Here is a snapshot, with a few action items. In this write-up of my current thinking, I'll use these terms: patchset:: Output from a single git diff invocation, which may contain one or more patch. patch:: Part of a patchset from a line that begins with diff --git up to (but excluding) the next line that begins with diff --git. tree:: diff compares two collections of files; each collection is a tree. Left and right sides of the comparison are called old tree and new tree, respectively. The tree we attempt to apply a patchset is the target tree. The tree we would get after successful application of a patchset is the resulting tree. A tree does not have to be a tree object--we may be comparing the index and the files in the working tree, for example. preimage:: postimage:: Lines in a patch that are prefixed by - or but not + that denote what the patched file ought to have for the patchset to apply (preimage). Lines with + or denote what the patch result ought to look like (postimage). 1. Basics First the basics. Let's think about a patchset with a single patch. What does this patchset tell us? diff --git a/major-08.txt b/major-08.txt index 680c5f6..5de90cb 100644 --- a/major-08.txt +++ b/major-08.txt @@ -1,3 +1,3 @@ -8. Fortitude. +8. Strength. This is one of the cardinal virtues, of which I shall speak later. It obviously tells us that the new tree changed Fortitude, that used to be in the old tree, to Strength, but it actually tells us a bit more about the old tree. For this patch to apply, the target tree must have a file named major-08.txt that begins with lines we see as the preimage in the patch. 2. Renaming a file Now let's get a bit fancier and study a patchset with a rename patch, which I invented very early in the Git's life (it was done while Linus was still running the project). What does this patchset tell us? diff --git rws/major-08.txt marseille/major-11.txt similarity index 97% rename from major-08.txt rename to major-11.txt index 680c5f6..2ab22a0 100644 --- rws/major-08.txt +++ marseille/major-11.txt @@ -1,3 +1,3 @@ -8. Fortitude. +11. Fortitude. This is one of the cardinal virtues, of which I shall speak later. We can see that this is going from the same old tree as the previous one's old tree, renames major-08.txt to major-11.txt with slight modification. It tells us a lot more about the trees, compared to the previous example. For this patchset to apply, the target tree must satisfy the same preconditon as the previous one about major-08.txt, and in addition it must lack major-11.txt; otherwise we wouldn't be renaming a new file to it. Also, it tells us that the resulting tree must lack major-08.txt. So far, these are straight-forward. In summary: rule 1. a patch from file A to file A means file A exists in the target tree with contents that match the preimage of the patch. file A will be in the resulting tree. rule 2. a patch renaming file A to file B requires that file A exists the target tree with contents that match the preimage of the patch, and file B does not exist in the target tree. file A will not exist in the resulting tree. rule 3. a patch that creates file A requires that file A does not exist in the target tree. rule 4. a patch that deletes file A requires that file A exists in the target tree with contents that match the preimage of the patch. The latter two I didn't illustrate with examples, but they should be obvious. 3. First twist: cross renaming Now, here is the first twist. What does this patchset mean? diff --git rws/major-11.txt marseille/major-08.txt similarity index 99% rename from major-11.txt rename to major-08.txt index 517d9f8..44e8d3a 100644 --- rws/major-11.txt +++ marseille/major-08.txt @@ -1,3 +1,3 @@ -11. Justice. +8. La Justice That the Tarot, though it is of all reasonable antiquity, is not of diff --git rws/major-08.txt marseille/major-11.txt similarity index 97% rename from major-08.txt rename to major-11.txt index 5de90cb..a101d5f 100644 --- rws/major-08.txt +++ marseille/major-11.txt @@ -1,3 +1,3 @@ -8. Strength. +11. La Force This is one of the cardinal virtues, of which I shall speak later. This is a swap patchset, that swaps major-08.txt and major-11.txt with small edit. You would have done something like this to prepare it: $ mv major-11.txt tmp $ mv major-08.txt major-11.txt $ mv tmp major-11.txt $ edit major-08.txt major-11.txt ;# just a little $ git commit -m swap major-08.txt major-11.txt $ git diff -B -M HEAD^ A patch renaming major-11.txt
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
Jeff King p...@peff.net writes: That is one of the reasons why I had the unbounded set, including the ones under our control such as subcommand names in the draft update for the guideline. I dropped that part after the discussion to keep other obviously agreed parts moving, but we may have to revisit it later. I think this may be the heart of where we were disagreeing. I took unbounded set to mean a set where you might keep adding things forever. So fsck errors would count in that. But if you mean it as a set where the syntax may be unbounded, then yeah, we definitely would not want it in the key name, as that becomes an unnecessary restriction. What I mean is possible keys are unbounded and its syntax is not under control of the 'config' subsystem. The syntax does not have to be unbounded; as long as it is wrong for the config subsystem to dictate what shape the possible values may take, it shouldn't be used as the top or the bottom level in the variable namespace where it has its own syntax restriction that may or may not match the requirement of the using code of the config subsystem. Those who name Git subcommands will be limited to sane looking subcommand names that do not have SP in it, for example, but just because config subsystem does not want to see _ in its keys, it should not force its world view to those who name subcommands. If the names are not unbounded, it becomes easier to live with such a third-party limitation (imposed by config subsystem), but otherwise, we just pick a name within that syntax becomes an unnecessary and artificial limitation. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
Jeff King p...@peff.net writes: A list of enum-like values where we are OK confining the names to the alnums is OK to use as an unbounded set of key values. Just like we have color.branch.*, we just pick a name within that syntax for any new values we add (and that is not even a burden; alnum names are what we would have picked anyway). I would say that color.branch.slot names are very different from subcommand names. The latter is exposed to the end users who do not have to know that they can be used and must be usable as config keys. color.branch.slot names were invented _only_ to be used to interact with the config, and nowhere else. Of course you can just pick a name within that syntax for configuration variables and be happy with it, because the users are very aware that they are using that name to name a configuration variable. The names of the subcommands are very different in that they are not just for accessing configuration variables---if the user does not have pager.cmd, the user will not use it as configuration keys anywhere in the system. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Sat, Feb 07, 2015 at 01:03:15AM +0100, Mikael Magnusson wrote: On Fri, Feb 6, 2015 at 8:44 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: On Fri, Feb 06, 2015 at 01:45:28PM +0100, Andreas Krey wrote: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? I reported this issue a few months ago, http://permalink.gmane.org/gmane.comp.version-control.git/258886 Someone sent a patch that never went anywhere, http://comments.gmane.org/gmane.comp.version-control.git/258895 Thanks. I had thought this all seemed familiar, and I did find your report in the archive, but not the follow-up patch[1]. It looks like that patch just squelches the error message. That fixes the immediate error-message regression, but does not fix the larger problem (that you cannot have an alias with an underscore, or set the pager config for a command with an underscore). But it is at least a start, and unless somebody is excited about taking it further, maybe it is enough for now. The thread ended with Tanay mentioning that new patches would be forthcoming. I've cc'd him, so hopefully that can still happen. -Peff [1] This is a good lesson in why it is nice to make sure that the in-reply-to headers for patches are set properly; it makes it easier later on to find related parts of the discussion. This is something I think that git-send-email doesn't make especially easy. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Fri, Feb 06, 2015 at 02:27:31PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: A list of enum-like values where we are OK confining the names to the alnums is OK to use as an unbounded set of key values. Just like we have color.branch.*, we just pick a name within that syntax for any new values we add (and that is not even a burden; alnum names are what we would have picked anyway). I would say that color.branch.slot names are very different from subcommand names. The latter is exposed to the end users who do not have to know that they can be used and must be usable as config keys. Yeah, again, sorry if I wasn't clear. That was the same contrast I was making. Of the examples given in this thread, color.branch.slot and fsck.* names are in one boat (OK to give them configuration-friendly names, they are just a list) and arbitrary commands are in another. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Fri, Feb 6, 2015 at 8:44 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: On Fri, Feb 06, 2015 at 01:45:28PM +0100, Andreas Krey wrote: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? I reported this issue a few months ago, http://permalink.gmane.org/gmane.comp.version-control.git/258886 Someone sent a patch that never went anywhere, http://comments.gmane.org/gmane.comp.version-control.git/258895 -- Mikael Magnusson -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Fri, Feb 06, 2015 at 01:45:28PM +0100, Andreas Krey wrote: there seems to be a regression in the behaviour of 'git show_ref' (note the underscore). In v2.0.3-711-g586f414 it starts to say: $ ./git show_ref error: invalid key: pager.show_ref git: 'show_ref' is not a git command. See 'git --help'. and somewhere (probably two commits, judging the diffs) later that changes again to: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? This is highlighting the problem with pager.* that Junio mentioned recently, which is that the keyname has arbitrary data, but syntactically is limited to alnum and -. This should have been: pager.show_ref.enabled from the beginning. But of course it was not. Even if we transition, we would want to support pager.* for a while. I don't think squelching the messages is quite the right approach. They come from git_config_parse_key, which barfs on parsing the syntactically invalid keyname. So not only are we complaining, but we are not actually looking up the value. I don't think that's technically a regression in 586f414, though. The reader started to complain, but AFAICT git would not agree to parse a file containing: [pager] show_ref = true in the first place. So it is not a new problem, but it is a bug that you cannot set pager config for such a command or alias. I can think of a few possible paths forward: 1. Squelch the messages, and declare show_ref and friends out-of-luck for pager config or aliases. 2. Relax the syntactic rules for config keys to allow more characters. We cannot make this perfect (e.g., we cannot allow . for reasons of ambiguity), but I imagine we could cover most practical cases. Note that we would need the matching loosening on the file-parsing side. 3. Start phasing in pager.*.enabled (and I guess pager.*.command). We would still do the lookup of pager.* for backwards compatibility, but we would be careful to do so only when it is syntactically valid. IOW, this looks like (1), except the path forward for show_ref is to use the new, more robust, syntax. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
On Fri, Feb 06, 2015 at 12:14:35PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: This is highlighting the problem with pager.* that Junio mentioned recently, which is that the keyname has arbitrary data,... Yes, even if it is not arbitrary (imagine we limit ourselves to the official set of commands we know about), the naming rule for the git subcommand names should not be dictated by the naming rule for the configuration variables, as they are unrelated. That is one of the reasons why I had the unbounded set, including the ones under our control such as subcommand names in the draft update for the guideline. I dropped that part after the discussion to keep other obviously agreed parts moving, but we may have to revisit it later. I think this may be the heart of where we were disagreeing. I took unbounded set to mean a set where you might keep adding things forever. So fsck errors would count in that. But if you mean it as a set where the syntax may be unbounded, then yeah, we definitely would not want it in the key name, as that becomes an unnecessary restriction. A list of enum-like values where we are OK confining the names to the alnums is OK to use as an unbounded set of key values. Just like we have color.branch.*, we just pick a name within that syntax for any new values we add (and that is not even a burden; alnum names are what we would have picked anyway). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Feb 6, 2015, at 12:05, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: Actually I just tested it. If we #undef it we could end up producing these: error: syntax error before DEPRECATED_ATTRIBUTE So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. Doesn't the fact that your test failed indicates that it is not jsut to be safe in case but is required for correctness? The first hit for MAC_OS_X_VERSION_MIN_REQUIRED was this: https://opensource.apple.com/source/CarbonHeaders/CarbonHeaders-18.1/AvailabilityMacros.h which marks quite a many macros to that value. I do not know what changes they make to openssl/*.h (which is included just after the above header is included, but I would imagine that is where the AVAILABLE_MAC_OS_X_VERSION_XXX_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_VERSION_YYY macros are checked and annoying warnings that are being squelched by the previous change are given? Yes. Although Eric didn't specify exactly where when he suggested adding this: On Feb 6, 2015, at 02:00, Eric Sunshine wrote: #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif I took the suggestion to be after the openssl/*.h headers are included which would avoid the error of having DEPRECATED_ATTRIBUTE be #undef'd for them. But, even math.h can end up including AvailabilityMacros.h, so I think #undef'ing DEPRECATED_ATTRIBUTE after the openssl/*.h headers are included would be unsafe in general. While we might happen to get away with that today, if say compat/apple-common- crypto.h changes in the future (or for that matter any sequence of includes in other files or any headers in the Apple SDK) we could start seeing the error. TLDR; yeah, DEPRECATED_ATTRIBUTE needs to remain defined to nothing. -Kyle -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
MAC_OS_X_VERSION_MIN_REQUIRED may be defined by the builder to a specific version in order to produce compatible binaries for a particular system. Blindly defining it to MAC_OS_X_VERSION_10_6 is bad. Additionally MAC_OS_X_VERSION_10_6 will not be defined on older systems and should AvailabilityMacros.h be included on such as system an error will result. However, using the explicit value of 1060 (which is what MAC_OS_X_VERSION_10_6 is defined to) does not solve the problem. The changes that introduced stepping on MAC_OS_X_VERSION_MIN were made in b195aa00 (git-compat-util: suppress unavoidable Apple-specific deprecation warnings) to avoid deprecation warnings. Instead of blindly setting MAC_OS_X_VERSION_MIN to 1060 change the definition of DEPRECATED_ATTRIBUTE to empty to avoid the warnings. This preserves any MAC_OS_X_VERSION_MIN_REQUIRED setting while avoiding the warnings as intended by b195aa00. Signed-off-by: Kyle J. McKay mack...@gmail.com --- git-compat-util.h | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index eb9b0ff3..0efd32c4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -212,12 +212,15 @@ extern char *gitbasename(char *); #endif #ifndef NO_OPENSSL +#ifdef __APPLE__ #define __AVAILABILITY_MACROS_USES_AVAILABILITY 0 -#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6 +#include AvailabilityMacros.h +#undef DEPRECATED_ATTRIBUTE +#define DEPRECATED_ATTRIBUTE +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY +#endif #include openssl/ssl.h #include openssl/err.h -#undef MAC_OS_X_VERSION_MIN_REQUIRED -#undef __AVAILABILITY_MACROS_USES_AVAILABILITY #ifdef NO_HMAC_CTX_CLEANUP #define HMAC_CTX_cleanup HMAC_cleanup #endif -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Fri, Feb 6, 2015 at 4:35 AM, Kyle J. McKay mack...@gmail.com wrote: MAC_OS_X_VERSION_MIN_REQUIRED may be defined by the builder to a specific version in order to produce compatible binaries for a particular system. Blindly defining it to MAC_OS_X_VERSION_10_6 is bad. Additionally MAC_OS_X_VERSION_10_6 will not be defined on older systems and should AvailabilityMacros.h be included on such as system an error will result. However, using the explicit value of 1060 (which is what MAC_OS_X_VERSION_10_6 is defined to) does not solve the problem. The changes that introduced stepping on MAC_OS_X_VERSION_MIN were made in b195aa00 (git-compat-util: suppress unavoidable Apple-specific deprecation warnings) to avoid deprecation warnings. Instead of blindly setting MAC_OS_X_VERSION_MIN to 1060 change the definition of DEPRECATED_ATTRIBUTE to empty to avoid the warnings. This preserves any MAC_OS_X_VERSION_MIN_REQUIRED setting while avoiding the warnings as intended by b195aa00. Signed-off-by: Kyle J. McKay mack...@gmail.com Tested on 10.10.2 (Yosemite) and 10.5.8 (Snow Leopard). Reviewed-by: Eric Sunshine sunsh...@sunshineco.com More below... --- git-compat-util.h | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index eb9b0ff3..0efd32c4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -212,12 +212,15 @@ extern char *gitbasename(char *); #endif #ifndef NO_OPENSSL +#ifdef __APPLE__ #define __AVAILABILITY_MACROS_USES_AVAILABILITY 0 -#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6 +#include AvailabilityMacros.h +#undef DEPRECATED_ATTRIBUTE +#define DEPRECATED_ATTRIBUTE +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY +#endif #include openssl/ssl.h #include openssl/err.h -#undef MAC_OS_X_VERSION_MIN_REQUIRED -#undef __AVAILABILITY_MACROS_USES_AVAILABILITY DEPRECATED_ATTRIBUTE is a fairly generic name. Do we want to be extra careful and #undef it here to avoid potential unintended interactions with other #includes (Apple or not)? #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif On the other hand, we've already mucked with it, so #undef may be superfluous. #ifdef NO_HMAC_CTX_CLEANUP #define HMAC_CTX_cleanup HMAC_cleanup #endif -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
BUG: 'error: invalid key: pager.show_ref' on 'git show_ref'
Hi all, there seems to be a regression in the behaviour of 'git show_ref' (note the underscore). In v2.0.3-711-g586f414 it starts to say: $ ./git show_ref error: invalid key: pager.show_ref git: 'show_ref' is not a git command. See 'git --help'. and somewhere (probably two commits, judging the diffs) later that changes again to: $ git show_ref error: invalid key: pager.show_ref error: invalid key: alias.show_ref git: 'show_ref' is not a git command. See 'git --help'. Apparently we need to squelch this message from within git_config_get_* in this case? Still present in 2.3.0. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.3.0
On Thu, Feb 5, 2015 at 11:53 PM, Junio C Hamano gits...@pobox.com wrote: The latest feature release Git v2.3.0 is now available at the usual places. [...] * Git 2.0 was supposed to make the simple mode for the default of git push, but it didn't. (merge 00a6fa0 jk/push-simple later to maint). Maybe I'm misunderstanding what this does, but changing the push default was *the* backwards compatibility breakage we advertised for v2.0.0[1]. A lot of users (including myself) upgraded to v2.0.0 very carefully making sure that the common pattern of git push our users were using wasn't broken. But apparently that change isn't taking effect until now. If so I think this needs to be advertised a lot more prominently than buried down along with other miscellaneous fixes in the changelog. 1. https://git.kernel.org/cgit/git/git.git/tree/Documentation/RelNotes/2.0.0.txt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
