[RESEND]MSI:ACTION_AppSearchReg() should return backslash terminated paths - Solves bug 13838

[Massimo Del Fedele]

James Hawkins ha scritto:
> On Mon, Jul 7, 2008 at 1:56 PM, Massimo Del Fedele <max at veneto.com> wrote:
> 
> Listen, I don't want to come off as mean, but like you say yourself,
> you are not knowledgeable about msi or the Win32 API for that matter.
> AppSearchReg queries the specified registry entry for a directory to
> check for its existence.  You say, "in reg, where paths are
> un-slashed."  There is no such definition of paths in the registry.  A
> registry value contains whatever value the user set it to.
I obviously meant "values contained in registry that are paths", but it 
seemed to me clear enough how I said.
We are talking about values in registry which REPRESENTS paths, and are 
REPRESENTED in registry as not backslash terminated paths, as all 
registry contained paths that I'm aware of.
> 
> 
> Again, what does that have to do with anything?  This is the special
> MSI properties code...not the AppSearch action.  You are trying to
> relate all these disparate parts that are completely unrelated.
> 
That HAS to do with the other case, because AppSearchReg is used, among 
others, to retrieve paths from registry and  set corresponding local 
properties.
If you look deeply enough inside code (no time now to copy/paste it), 
you'll see that LOCAL path properties (as LOCALAPPFOLDER, for example) 
are get from registry using AppSearch and set up with its returning 
value. EXACTLY as my example reported before about the hard-coded global 
properties, where paths are read from registry, a backslash is appended 
and are stored as global properties. The ONLY difference is the missing 
"append-the-backslash" stuff when setting up local path properties.
I can see a STRONG relation between them, don't you ?
.................
> 
> By definition you can't know if it's correct unless you add tests for
> it, and even then you can never be 100% sure (but it's pretty close).
> 
You can't even with 100 tests. What you can have is a "reasonable 
knowledge" that it "should" be right.
What I've done in my (buggy indeed) patch was to limit intervention to 
just ONE problem.
>> Maybe it doesn't fix the universe, but it does fix a bug, and
>> just THAT bug, without fiddling with other stuffs.
> 
> ...and maybe it breaks some other apps.  You act like I don't think
> your fix is correct.  It probably is correct, but that's not the
> point.  It's not tested, so you can't be certain that it's correct.  I
> already said I'd add a test, so I don't know why you keep going on
> about this.
I do because I'm convinced that the way I solved is ONE of possible 
RIGHT ways, and because I've got tons of people that would like to run 
the autocad application without having to build wine from 
source...That's all. I still think it's better a partial fix that solves 
a problem for many people than wait for a 
"maybe-perfect-fix-in-another-year-or-so". I've seen tons of patches 
that caused regressions, you're right on it, but mine is so simple and 
straightforward that simply can't do harm, but solves a problem.
Nobody says that a "better" patch can't be applied in future.
I located the problem since... let's say 3 months ? as nothing was 
moving, I found an hack that solved the problem, at first, and, as 
nothing was moving on  again I added a patch that do the same without 
being an hack.
Autocad is one of most requested apps for wine, and nothing was moving 
since years... I'd say about 5-6 years, when I found some solution for 
R14 and 2000 versions.

> 
>> But maybe you wanted to say that is ACTION_SearchDirectory() that must
>> be patched ?
> 
> Yes.  I thought that was obvious, but I'll try to not make those
> assumptions again.
Me too. As I said, my patch solves ONE problem, not all related ones.
We're speaking about it since 3 weeks, now, and I still haven't seen a 
better solution nor a demonstration that my way is bad.
> 
>> Maybe that's right. BUT then, as it's not an exported
>> function, you should test ALL usages of it and see if that's a correct
>> behaviour. Well, It's used just in AppSearchDr, besides of
>> AppSearchReg(). Maybe, knowing what is for AppSearchDr() I could know if
>> it's correct to have its result backslashed too... But I don't. Again,
>> my skills on MSI are *very* limited. I've chosen to fix just something I
>> know must be fixed instead of try to fix the world without knowing
>> anything about it. Wrong ?
>>
> 
> Yes.  You're verifying everything I've told you several times.
> 
I already verified that the patch does no harm AND solves a problem.
For me that's more than enough.

Ciao

Max