On Thu, Feb 19, 2009 at 12:28 PM, Aric Stewart aric@codeweavers.com wrote:
Fix for office 2007 sp 1 install.
dlls/msi/media.c | 33 +++++++++++++++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-)
I believe you're going down the wrong road with respect to the Office 2007 SP1 installer. None of the files installed/patched by SP1 need access to the original media, or stored local copies of the cab archives. With the fullfile install (which I'm not sure there's another version), you should be able to remove the original media, erase the stored cab files, and successfully install SP1. The fact that our implementation tries to access those cabs or the original media is the bug. There are many conditions and variables that factor in to whether a file should be reinstalled or not, and most of them have been fixed, but this patch is hiding all of the rest of the bugs. I'm not sure if you got the email I sent Jer about this problem, but the real bug is that we aren't handling the REINSTALL variable correctly (or REINSTALLMODE for that matter.) Can you please describe the exact case for why this patch is needed?
Hello,
No i never saw the e-mail that you send to Jeremy.
I will admit that i did not dig really deep for this patch as it appeared to me that the fix was obvious (there was even a fixme in the code saying we needed to do what the installer was asking for)
There are about 4 CAB files that the service pack asks for to install (and only installs a few files out of each) that are accessed by this method. Each one does not exist on the original media at all and only seems accessable via the LastSource location.
I am curious what tests you did to prove that you did not need the original media or even the copied cab files? Did you do a test on windows where you deleted them all and the upgrade worked?
I will investigate some more, but i do not feel that this patch is incorrect, even if it should not be needed. Am I right there?
thanks, -aric
James Hawkins wrote:
On Thu, Feb 19, 2009 at 12:28 PM, Aric Stewart aric@codeweavers.com wrote:
Fix for office 2007 sp 1 install.
dlls/msi/media.c | 33 +++++++++++++++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-)
I believe you're going down the wrong road with respect to the Office 2007 SP1 installer. None of the files installed/patched by SP1 need access to the original media, or stored local copies of the cab archives. With the fullfile install (which I'm not sure there's another version), you should be able to remove the original media, erase the stored cab files, and successfully install SP1. The fact that our implementation tries to access those cabs or the original media is the bug. There are many conditions and variables that factor in to whether a file should be reinstalled or not, and most of them have been fixed, but this patch is hiding all of the rest of the bugs. I'm not sure if you got the email I sent Jer about this problem, but the real bug is that we aren't handling the REINSTALL variable correctly (or REINSTALLMODE for that matter.) Can you please describe the exact case for why this patch is needed?
On Fri, Feb 20, 2009 at 4:45 AM, Aric Stewart aric@codeweavers.com wrote:
Hello,
No i never saw the e-mail that you send to Jeremy.
I will admit that i did not dig really deep for this patch as it appeared to me that the fix was obvious (there was even a fixme in the code saying we needed to do what the installer was asking for)
Unfortunately the error is misleading. A good mentality to take is that if you see the error about 'unable to extract ABC.cab' then we're trying to install a file that shouldn't be installed (or reinstalled.)
There are about 4 CAB files that the service pack asks for to install (and only installs a few files out of each) that are accessed by this method. Each one does not exist on the original media at all and only seems accessable via the LastSource location.
I don't see how that's possible. If they're in LastSource, that means we used them before and came from the original media. I'm confused about that though, because the only cabs that should be accessed by the SP1 installer are stored in the package itself. Any other file that tries to be installed is improperly being reinstalled.
I am curious what tests you did to prove that you did not need the original media or even the copied cab files? Did you do a test on windows where you deleted them all and the upgrade worked?
Yes that is one way to test it. The fullfile SP1 does not, and should not, rely on the original media and stored cabs. If our msi is asking for those old cabs, it means we're reinstalling a file that shouldn't be reinstalled (which is a bug.)
I will investigate some more, but i do not feel that this patch is incorrect, even if it should not be needed. Am I right there?
No, I think the patch is wrong in that if you remove the original media and the stored cabs, the install will fail.
-- James Hawkins
Ok i am following you but.
James Hawkins wrote:
On Fri, Feb 20, 2009 at 4:45 AM, Aric Stewart aric@codeweavers.com wrote:
Hello,
. . .
No, I think the patch is wrong in that if you remove the original media and the stored cabs, the install will fail.
By that thinking then the whole function find_published_source is a horrible hack and should be removed as being wrong. It is not the case that these published sources should be accessible?
Maybe this is not the correct fix for SP1. But is this patch incorrect for msi also?
I am looking at the REINSTALL logic and it looks like it may be tricky.
-aric
On Fri, Feb 20, 2009 at 9:56 AM, Aric Stewart aric@codeweavers.com wrote:
Ok i am following you but.
James Hawkins wrote:
On Fri, Feb 20, 2009 at 4:45 AM, Aric Stewart aric@codeweavers.com wrote:
Hello,
. . .
No, I think the patch is wrong in that if you remove the original media and the stored cabs, the install will fail.
By that thinking then the whole function find_published_source is a horrible hack and should be removed as being wrong. It is not the case that these published sources should be accessible?
Na it's not a hack. Some patches require the old source to be available (older versions of Office patches for example.) But those all work fine, so I can't say whether the patch is correct or not without tests. My main concern is that we're now *hiding* other bugs because of the changed behavior.
Maybe this is not the correct fix for SP1. But is this patch incorrect for msi also?
I am looking at the REINSTALL logic and it looks like it may be tricky.
I think you're best bet is to append tests to tests/package.c:test_states() that set REINSTALL and REINSTALLMODE to various values and see what the states are. From there the necessary implementation should be apparent. The most important part of REINSTALL(MODE) is that it allows an already installed non-versioned file to not be reinstalled (in certain cases.) If I remember correctly, we needed this one for SP1. Also, REINSTALL is set to the features that are requested to be reinstalled, and we shouldn't try to reinstall anything else in that situation.
-
Aric Stewart -
James Hawkins