Opened 15 years ago

Closed 14 years ago

#3762 closed bug (fixed)

PackageInstaller and handling script files

Reported by: sil2100 Owned by: sil2100
Priority: low Milestone: Unscheduled
Component: Applications/PackageInstaller Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

There were no problems reported regarding this matter yet, but I presume some packages use this feature. Right now we treat them as normal files.

Attachments (2)

script.patch (35.9 KB ) - added by sil2100 14 years ago.
Patch fixing the invalid script file handling + some other modifications.
fixed_script.patch (36.4 KB ) - added by sil2100 14 years ago.
Fixed according to the notes provided by korli, thanks! Also fixed something I forgot about earlier. (v3)

Download all attachments as: .zip

Change History (13)

comment:1 by kaliber, 14 years ago

I need this functionality. Is there any chance to implement it? This should not be difficult in my opinion.

comment:2 by sil2100, 14 years ago

Status: newin-progress

It shouldn't. I will take care of this in the nearest 3-4 days, since right now I have too much workload.

comment:3 by sil2100, 14 years ago

I'm sorry for the delay, I only recently found enough time to start fixing this problem. Could anyone attach/paste a link to a real-life package using this functionality? I would like to test the changes.

comment:4 by diver, 14 years ago

IIRC cpu_fix used this feature http://www.bebits.com/app/3999

by sil2100, 14 years ago

Attachment: script.patch added

Patch fixing the invalid script file handling + some other modifications.

comment:5 by sil2100, 14 years ago

This patch should do it. I used the occasion to export the installation procedure to another thread, since earlier the application was simply not responding during file copy. Checked it on the cpu_fix package.

Would be grateful if anyone could check if everything works fine with this patch. Thanks!

comment:6 by korli, 14 years ago

A few things I noticed:

  • ParseScript and RunScript should be prefixed with an underscore.
  • if inflate_data fails, the allocated *script array is not deleted.
  • code style problems, example PackageInstall.cpp line132: } <newline> else {
  • the macro RETURN_MESSAGE() is a problem IMO. I would add a method which calls Install and post the message if needed.

comment:7 by sil2100, 14 years ago

Thanks for reviewing the patch, korli!

I have fixed the bugs you have highlighted in the new patch attached below. I have added the deletion of *script memory in _ParseScript(), although it was not really necessary - the memory allocated in *scipt during _ParseScript() was later freed in the DoInstall() method (if, of course, the inflate_data function failed). But your approach seems more consistent in this case.

I am also sorry for the style problems - I am trying to proof read the code before submition, but it's really hard to get rid of some coding habits. Hope I did not miss anything.

comment:8 by sil2100, 14 years ago

I noticed that the *script parameter was a reminder of some other, older idea that did not end up in the final patch - so I removed it and made script a local variable for the _ParseScript() method. Attaching modified patch (the same name as before).

comment:9 by korli, 14 years ago

Another question, why adding "#include <Locker.h>" to PackageView.h ?

comment:10 by sil2100, 14 years ago

Ah, that's another bogus leftover, sorry for that again. This was added because at first I wanted some parts of the new lock-needing functionality in PackageView, but changed my mind at the end. It seems I forgot to remove the include.

I'm re-uploading the patch once again.

by sil2100, 14 years ago

Attachment: fixed_script.patch added

Fixed according to the notes provided by korli, thanks! Also fixed something I forgot about earlier. (v3)

comment:11 by korli, 14 years ago

Resolution: fixed
Status: in-progressclosed

Applied patch in hrev36020.

Note: See TracTickets for help on using tickets.