Patch for boolean support for Protocols.XMLRPC
--- module.pmod.orig 2011-08-31 16:02:19.838058384 -0400 +++ module.pmod 2011-09-07 10:17:21.848427570 -0400 @@ -15,12 +15,15 @@ //! Pike @expr{mapping@} is translated to XML-RPC @tt{<struct>@}. //! Pike @expr{array@} is translated to XML-RPC @tt{<array>@}. //! Pike @[Calendar] object is translated to XML-RPC @tt{<dateTime.iso8601@}. +//! Pike @expr{Val.false@} and @expr{Val.true@} is translated to +//! XML-RPC @tt{<boolean>@}. //! //! Translation rules for conversions from XML-RPC datatypes to Pike //! datatypes: //! -//! XML-RPC @tt{<i4>@}, @tt{<int>@} and @tt{<boolean>@} are -//! translated to Pike @expr{int@}. +//! XML-RPC @tt{<i4>@} and @tt{<int>@} are translated to Pike @expr{int@}. +//! XML-RPC @tt{<boolean>} is translated to Pike @expr{Val.true@} and +//! @expr{Val.false@}. //! XML-RPC @tt{<string>@} and @tt{<base64>@} are translated to //! Pike @expr{string@}. //! XML_RPC @tt{<double>@} is translated to Pike @expr{float@}. @@ -225,8 +228,9 @@ return sizeof(data)?data[0]:""; case "i4": case "int": - case "boolean": return (int)(data*"") || magic_zero; + case "boolean": + return ((int)(data*""))?Val.true:Val.false; case "double": return (float)(data*""); case "string": @@ -274,7 +278,7 @@ ({ "&", "<", ">", """, "'", "�" })); }
-protected string encode(int|float|string|mapping|array value) +protected string encode(int|float|string|mapping|array|object value) { string r = "<value>"; if(intp(value)) @@ -303,6 +307,8 @@ else if (objectp (value) && value->format_iso_short) r += "<dateTime.iso8601>" + value->format_iso_short() + "</dateTime.iso8601>"; + else if (objectp(value) && (value->is_val_true || value->is_val_false)) + r += sprintf("<boolean>%d</boolean>",(int)value); else error("Cannot encode %O.\n", value); return r+"</value>\n";
The patch in decode() is good, but it isn't backward compatible since old code might expect the integers 0 and 1 for booleans. This is what we discussed earlier on whether or not to make the boolean objects int-like.
As I said earlier, I suggest an optional flag to Client/AsyncClient to enable the newstyle decode mode in responses, and to decode_call to enable it for incoming calls.
I also miss a corresponding patch in 7.9.
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sat, September 10, 2011 12:30:11 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
The patch in decode() is good, but it isn't backward compatible since old code might expect the integers 0 and 1 for booleans. This is what we discussed earlier on whether or not to make the boolean objects int-like.
As I said earlier, I suggest an optional flag to Client/AsyncClient to enable the newstyle decode mode in responses, and to decode_call to enable it for incoming calls.
I also miss a corresponding patch in 7.9.
Well, I've got a compat mode thing going, but the decode of the boolean (in the new patch) isn't working correctly. It probably wasn't in the previous patch either. Val.false won't get returned, because it is 0, and the documentation says 0 won't get returned, which it won't, it turns into an empty string (part of the xml parse). Probably because it prints as an int, even though it isn't. I presume Val.true works correctly, I haven't been able to test it yet. Trying to find some xmlrpc data from my server that will return true, but everything is false so far.
May have to be satisfied with false being returned as 0 instead of Val.false, and true being returned as Val.true as expected.
Perhaps someone can see where my error lies, if any?
How tiresome. :P
Now I see the magic_zero thingy is a workaround for the same thing. It wasn't the wisest choice to disregard any false value like that in the parser.
I think the best solution would be to fix Parser.XML.Simple.parse to use UNDEFINED as "disregard" value instead, since the current behavior clearly affects the usability.
But doing so would of course introduce another compat issue, and I don't know if a flag would be required there as well, or if we could just use the #pike compat level. I'd be somewhat in favor of the latter, but I don't really know how much that parser is used.
In any case, it wouldn't be good to extend the decoder to handle Val.true without handling Val.false as well. If you're not inclined to dig into the parser, and cannot find another passable workaround (by using another xml parser, or perhaps another way to collect the results, or something), then I think the best course is to not change anything on the decode side for now.
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sat, September 10, 2011 4:15:02 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
How tiresome. :P
Now I see the magic_zero thingy is a workaround for the same thing. It wasn't the wisest choice to disregard any false value like that in the parser.
I think the best solution would be to fix Parser.XML.Simple.parse to use UNDEFINED as "disregard" value instead, since the current behavior clearly affects the usability.
But doing so would of course introduce another compat issue, and I don't know if a flag would be required there as well, or if we could just use the #pike compat level. I'd be somewhat in favor of the latter, but I don't really know how much that parser is used.
In any case, it wouldn't be good to extend the decoder to handle Val.true without handling Val.false as well. If you're not inclined to dig into the parser, and cannot find another passable workaround (by using another xml parser, or perhaps another way to collect the results, or something), then I think the best course is to not change anything on the decode side for now.
I agree, but I fixed it by doing the same sort of thing. I created a magic_false class that gets replaced at the end with Val.false if the compatibility flag is off, or 0 (through the normal code) if it is on.
I've tested it on 7.8, and the module.pmod file is identical with 7.9, so it should work there too. I tested some of my routines with the compat flag both on and off, and verified that I got the results I expected. Everything should be good now.
Thanks for the effort, but it still needs some fixing:
The compat flag is the wrong way. The point with compat is that old code should continue to work without changes. With this patch it's still necessary to change some calls to get the old behavior back. (If you don't like the flag in new code then we could discuss controlling it with the pike compat level instead. It's a bit more cumbersome while migrating, but maybe less so later on.)
Afaik xmlrpc <struct>s and <array>s are fully recursive. It doesn't look like the magic_false replacement code handles that.
Shouldn't decode_call take the flag as well?
Still missing a corresponding checkin in 7.9, but it'll come when the dust has settled?
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sun, September 11, 2011 12:45:02 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
Thanks for the effort, but it still needs some fixing:
The compat flag is the wrong way. The point with compat is that old code should continue to work without changes. With this patch it's still necessary to change some calls to get the old behavior back. (If you don't like the flag in new code then we could discuss controlling it with the pike compat level instead. It's a bit more cumbersome while migrating, but maybe less so later on.)
Afaik xmlrpc <struct>s and <array>s are fully recursive. It doesn't look like the magic_false replacement code handles that.
Shouldn't decode_call take the flag as well?
Still missing a corresponding checkin in 7.9, but it'll come when the dust has settled?
Sure, actually, though, I did 7.9, but it looks like I just forgot to push it.
I will make those changes, so the compat is reversed.
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sun, September 11, 2011 12:45:02 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
Thanks for the effort, but it still needs some fixing:
The compat flag is the wrong way. The point with compat is that old code should continue to work without changes. With this patch it's still necessary to change some calls to get the old behavior back. (If you don't like the flag in new code then we could discuss controlling it with the pike compat level instead. It's a bit more cumbersome while migrating, but maybe less so later on.)
Afaik xmlrpc <struct>s and <array>s are fully recursive. It doesn't look like the magic_false replacement code handles that.
Shouldn't decode_call take the flag as well?
Still missing a corresponding checkin in 7.9, but it'll come when the dust has settled?
Actually, I don't think it is necessary that decode_call take the flag, because if you don't pass in any Val objects, then everything will be an int anyway. It currently isn't possible to encode boolean unless you use Val, so if you need to, you will need to use the new features.
I'm creating a recursive function to handle replacing magic_false.
I reversed the flag, called it boolean instead.
I tried to commit 7.9, but it is giving me an error:
Counting objects: 172913, done. Delta compression using up to 2 threads. Compressing objects: 100% (38656/38656), done. Writing objects: 100% (170600/170600), 25.22 MiB | 104 KiB/s, done. Total 170600 (delta 134386), reused 164359 (delta 129690) remote: Commit 65a4850d0c0afba9b89ba4698b728c5224c3ee97 does not contain d0344c6c82231970df581550111574da60aa8a84 in its first-parent ancestry. remote: Did you pull with merge instead of rebase? To git-pike@pike-git.lysator.liu.se:pike.git ! [remote rejected] 7.9 -> 7.9 (pre-receive hook declined) error: failed to push some refs to 'git-pike@pike-git.lysator.liu.se:pike.git'
remote: Commit 65a4850d0c0afba9b89ba4698b728c5224c3ee97 does not contain d0344c6c82231970df581550111574da60aa8a84 in its first-parent ancestry.
There's your error. d0344c6 is the current head of 7.9, but you are trying to push something (65a4850) which is not a continuation from that point. Please check your commit graph, and rebase as needed.
Actually, I don't think it is necessary that decode_call take the flag, because if you don't pass in any Val objects, then everything will be an int anyway. It currently isn't possible to encode boolean unless you use Val, so if you need to, you will need to use the new features.
Sounds like you're talking about encode_call, and you're right in that case. However, decode_call takes an xml string and returns a Call object, so it could also benefit from the new decoding feature. I believe decode_call is intended for incoming calls when one implements an xmlrpc server.
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sun, September 11, 2011 2:00:16 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
Actually, I don't think it is necessary that decode_call take the flag, because if you don't pass in any Val objects, then everything will be an int anyway. It currently isn't possible to encode boolean unless you use Val, so if you need to, you will need to use the new features.
Sounds like you're talking about encode_call, and you're right in that case. However, decode_call takes an xml string and returns a Call object, so it could also benefit from the new decoding feature. I believe decode_call is intended for incoming calls when one implements an xmlrpc server.
Hmm, you are right. Let me check the code again.
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sun, September 11, 2011 2:00:16 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
Actually, I don't think it is necessary that decode_call take the flag, because if you don't pass in any Val objects, then everything will be an int anyway. It currently isn't possible to encode boolean unless you use Val, so if you need to, you will need to use the new features.
Sounds like you're talking about encode_call, and you're right in that case. However, decode_call takes an xml string and returns a Call object, so it could also benefit from the new decoding feature. I believe decode_call is intended for incoming calls when one implements an xmlrpc server.
Yes, you are absolutely right there. I was just going through all the method calls the were referenced from client, not any others that were referenced externally. I now have all decode* methods accepting the boolean flag.
Since re-cloning 7.9 my commits are going fine. I don't know when it got off, but it seems to be good now, thanks.
Yes, looks ok now. Good. :)
Since re-cloning 7.9 my commits are going fine. I don't know when it got off, but it seems to be good now, thanks.
Recloning sounds overly brutal, but I can't say what's happened in your case. If it's the usual problem with that the branch head has moved on the server, so that you need to rebase, then it looks something like this:
git push
To git-pike@pike-git.lysator.liu.se:pike.git ! [rejected] 7.9 -> 7.9 (non-fast-forward) error: failed to push some refs to 'git-pike@pike-git.lysator.liu.se:pike.git' To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes (e.g. 'git pull') before pushing again. See the 'Note about fast-forwards' section of 'git push --help' for details.
Hmm, on second thought, I suspect your problem is that you merge instead of rebase on "git pull". Check that you've got the rebase flag set to true, i.e. your .git/config should contain something like this:
[branch "7.9"] remote = origin merge = refs/heads/7.9 rebase = true
----- Original Message ----
From: "Martin Stjernholm, Roxen IS @ Pike developers forum" 10353@lyskom.lysator.liu.se To: pike-devel@lists.lysator.liu.se Sent: Sun, September 11, 2011 3:05:03 PM Subject: Re: Val.true and Val.false [Was: XMLRPC] (from pike@roxen.com)
Hmm, on second thought, I suspect your problem is that you merge instead of rebase on "git pull". Check that you've got the rebase flag set to true, i.e. your .git/config should contain something like this:
[branch "7.9"] remote = origin merge = refs/heads/7.9 rebase = true
Yes, that was it. Is that documented anywhere, or is that something I should have known? This is the only thing I use git with, for my personal projects I use mercurial.
Not really. http://pike.ida.liu.se/development/git/rw-git.xml ought to describe how to set it up for rebasing.
I could write a few words about it there, if someone fixes me up with an account on the Roxen server. Either I don't have one, or I've no idea what the password could be.
Okay, I got it. I just recloned the repo and recommited. 7.8 and 7.9 should have the patches in now.
pike-devel@lists.lysator.liu.se
-
Lance Dillon -
Marcus Comstedt (ACROSS) (Hail Ilpalazzo!) ＠ Pike (-) developers forum -
Martin Stjernholm, Roxen IS ＠ Pike developers forum