roxen.lists.roxen.general

Subject Author Date
Re: Lots of patches that can be integrated into Roxen 5.0 in a heartbeat Stephen R. van den Berg <srb[at]cuci[dot]nl> 09-01-2009
Martin Stjernholm wrote:
>"Stephen R. van den Berg" <<srb[at]cuci.nl>> wrote:
>> commit 4277b4905222032c8fb5a0eab460f60d39f78ead
>>     Housekeeping
>> Supplies a suitable .gitignore.

>Ok, although I don't know what *.s and *.a would match.

Just precaution, in case parts of the build process would start leaving
those behind.  The *.s files are a natural result of certain gcc flags
which cause the compiler to leave behind the assembler files.

>> commit 792000c527e0a6567ecba48934e3211568530c97
>>     data sometimes is empty
>> Fixes a crash I had sometimes, not sure if this is needed.

>If that fails then the error is elsewhere. An improvement would be to
>throw the I/O error reported through errno instead.

The problem occurred more than four years ago, I'll simply remove the patch
and see what happens nowadays.

>> commit baaca0f710c9a95ac5921f5b9d1fd44d26b4fa34
>>     Saner default for mysql querycache
>> Roxen should do the caching, not mysql.

>Not sure about the repercussions to change this. There are many
>situations when a Roxen level cache isn't viable (e.g. external
>clients, or because it'd be too much extra effort to write one).

External clients on the local mysql database of Roxen?
Is that something that happens a lot in your applications?
With respect to "too much effort"...
The MySQL query cache is useful if one has queries with longer than
trivial runtimes and comparatively small resultsets.  The way Roxen
uses the local MySQL database, this never happens.  The queries:
- Either have very short runtimes because they are indexed properly.
- Or if they run longer, then the query is some kind of status/maintenance
  query which is not intended to be called a lot in rapid succession.
- Or have very large resultsets, but should not be cached in that case,
  since they are not performance-critical (the problem is the large
  resultset to begin with).

So that would imply that maintaining the query cache in MySQL is close to
useless for the Roxen local database.

Any caching needed will be done by the OS due to filesystempages left in RAM.
Caching in MySQL as well, will just keep around that data in memory *twice*.

>> commit ad8a92ec5316e71b71638558c077e3b61ce92afd
>>     Fix longstanding errors in the quoting of <redirect>
>> I'm not entirely certain anymore that they are/were errors, someone needs
>> to seriously look at this and reevaluate the fix.  If it does fix a bug,
>> it is seriously needed.  It's a one-line patch, so it's easy to
>> check.

>I think the original code is correct. http_encode_invalids doesn't
>double-quote, so it's safe to use on an already encoded url.

The reason I took it out, is because it *seemed* impossible to specify
certain URLs.  I'll try and dig up some old examples which seemed impossible
to compute using RXML only.

>> commit 8b7de561ead32a21f50b8508a5735de801cc421a
>>     Make <redirect> work without loosing query arguments
>> This fixes a bug.

>The patch doesn't handle port paths correctly, I think (base_url() can
>return a path component after the host:port/ part). Judging from the
>your commit message, I've now (in cvs rev 1.50) fixed the problem this
>patch intends to fix, though.

Will test this, seems to look ok, thanks.

>> commit b4b503c88cf69d991e23acf4598e14686d635f39
>>     Prevent cimg from asking for username/password at times

>Don't understand what the bug is that this would solve.

If I recall correctly, the problem was/is that:
- A user accesses a cimg from inside a password protected area of the tree.
- cimg sees the image is not present in the database, so it converts the
  source image and stores a copy *alongside* with the user/password
  requirements of the original image.
- The cimg-URL produced is unguessable (or should be), but outside the
  password protected subtree of the original image.
- Now the browser uses this cimg-URL and is surprised by a not-authorised
  message from the server.  The browser doesn't have a username/password
  to supply because it's from outside the previous tree where it had already
  cached that information and presents a box to the user.
- That was one username/password check too many.  The cimg-URL should
  be username/password check free, because the URL should only be
  available to someone already able to get at the original image.

>> commit cd3cd82a2af0d19ddb060a561d296e8ca6f308cb
>>     Take the local tree out of the repository proper

>I don't think we want this. Those files are there to provide a
>skeleton local dir hierarchy and some help.

How about storing that local tree on a different directory in the source
management tree, and in the install script, simply copy it into the proper
spot?  The problem is that there are ../local paths in quite a lot of places,
which makes a live sourcetree a liability, because the local tree should
not be managed by git in this case.

>But the _NewLDAP.pmod tree is no longer in our cvs. It's probably a
>"dirty" delete that you haven't detected in your git import.

That should show up when I do a final run on the Roxen repository using the
same integrity checking scripts as I'm making for the Pike tree, but that
is something for after the Pike tree has been solidified.

>> commit caae472b3e0d448a929ce9a2dd8fa64be5ce3969
>>     Cater for lost connections during the last query

>Is there something that destructs the sql connection object on error?
>Otherwise con will never be zero there.

There are some places, yes, though I might be responsible for those, I'll
reevaluate why I did that, and maybe it can be fixed at that end instead.

>> commit 26ca63251a023c65a6468c8d9f1c52077caddc48
>>     Alternatives don't always contain an array

>Have you gotten a real error there? The plan is that the mapping
>contains only arrays when a timeout is set, and otherwise it shouldn't
>get into that loop.

Yes, got real errors there.  Though they might have been a result of
a possibly imperfect cucimorph filesystem (a filesystem which is
completely projected onto a PostgreSQL database).  I'll put this on
my own todo list again to see if it is really necessary or can be fixed
in a different spot.

>> commit 1b884b993f29e7ba9ca580905e5a548c71e08b10
>>     Add support for none encoding in <replace>

>Guess one could possibly want that feature, but I don't think "quote"
>is a particularly good attribute name - it gives the impression that
>there would be more than quoting scheme to choose from. And docs are
>missing.

>Also, in 5.0 the same thing should be achievable using something like
>this (untested):

>  <value type="text/plain"><replace ...>...

>I'm inclined to consider that good enough since it looks like a
>marginal feature.

If that works, it'll be enough, and I don't need this patch anymore.
I'll test that.

>> commit ea7fa0db8a2c0b76984c0f3c418cf6655d53f991
>>     Add support for mysql-rxml type quoting

>This quoting is dubious - if that is needed then I suspect there's a
>double-parse problem elsewhere.

Not quite.
Picture this:
<eval>
<maketag name=emit type=container>
<attrib name=host>&var.db;</attrib>
<attrib name=source>sql</attrib>
<attrib name=query>
SELECT abc
FROM def
WHERE xyz='&form.&var._fieldid;:mysql-rxml;'
</attrib>
....
</maketag>
</eval>

>> commit e5106525998e41b72d9aae0306a70de1b33b82d2
>>     Add support for csv quoting

>This looks like a useful feature, but it needs good docs first. I'm
>not gonna write that for you, sorry.. ;)

>Although I haven't looked at it in detail, the stuff in TagEmitValues
>looks slow with single-stepping chars in pike. I'm pretty sure it can
>be sped up considerably using some sscanf tricks.

I'll whip up some docs and peek into the sscanf speedup.
-- 
Sincerely,
           Stephen R. van den Berg.

"People who think they know everything are annoying to those of us who do."