[lokisetup] plugins in SETUP_2_0...

[Stéphane Peter]

On Jun 5, 2005, at 10:22 PM, Ryan C. Gordon wrote:
> I suspect Stephane is going to rip the UI a new asshole for 2.0, so  
> I spent some time thinking about the plugins instead.
>
> Overall, I'm pleased with how the 1.0 archive system worked out in  
> principle, but in implementation it has a lot of the same problems  
> as the UI layer: code duplication, assumptions, and generic higher- 
> level burdens. There are various generic tags that each plugin is  
> required to cut and paste from existing plugins, and some do and  
> some don't...plus there are UI callbacks passed around, etc.

I agree. While the plugin system we have now mostly works, its API  
has become way too encumbered because of repeated changes to the file  
copying code, to which it is currently tied.

> It is exceedingly difficult to implement a new plugin, even for  
> people that have done it before. When a new XML attribute is added  
> to the codebase, it requires editing each plugin to support generic  
> functionality. Working with the plugin code tends to be error-prone  
> in this respect.

Agreed.

> So what we need, I think, is basically this:
>
> 1) Let's try to keep what's there intact in terms of  
> functionality...we have perfectly good zip, tar, rar, etc handlers,  
> it doesn't make sense to throw that existing code away entirely.
> 2) Let's try to have all the high-level code exist in the base  
> loki_setup, and make sure the plugins only deal with their archive.  
> They should do one thing well.
> 3) Let's have copying files and directory trees be a plugin so it's  
> not a special case anymore.
> 4) Let's get rid of the callbacks.

Sounds good. The callbacks in the plugin API, as I said, are a  
vestige of the old file-copying code, and used solely to provide a  
way for the UI to be updated while the files are being extracted. If  
we're going to rewrite the UI code, it makes sense to completely get  
rid of those anyway.

> Basically, the plugins shouldn't be a one entry point "unpack this  
> file and get back to me" thing. Ideally, they should just look more  
> or less like the regular file code...the high level sees what files  
> are there, and then copies them, block by block.

This could be a valid approach. Of course, the rewrite of the plugin  
system will have to be done at the same time as a the rewrite of the  
whole I/O system, so we'll see exactly how this works out, but i like  
the idea of making regular files being handled as just another  
plugin. It will promote healthier code.

> The plugin interface then looks something like this (naming  
> convention is, of course, just an example here):
>
> (these would be function pointers in a struct...)
>
>
> First, find the plugin to handle a file by iterating over all  
> plugins with this method...
>
> // Return non-zero if this is the .XXX plugin and this is really  
> an .XXX
> // file. Where possible, plugin should determine this by file  
> signature // (.zip files have end-of-central-directory, etc).  
> Failing that, the
> // plugin can report true or false based on the filename extension.
> // In most cases, this would obsolete the "file" tag's "suffix"
> // attribute.
> int canHandleThisFile(const char *fname);
>
>
>
> Once you find the right plugin, open the archive...
>
> // prepare plugin to unpack this archive. Returns NULL on failure, an
> // opaque data type on success.
> void *openArchive(const char *fname);
>
>
>
> Once the archive is opened, you iterate through its contents with  
> getNextItem() and readMoreData() until you are done...
>
> // Get the next item from the archive. It may be a file, symlink, dir,
> //  or some other thing.
> //
> // PluginNextItemInfo is a struct like this:
> //  struct {
> //      const char *itemName;  // name of item ("path/myfile.txt",  
> etc)
> //      NextItemEnum itemType;  // file, dir, symlink, etc.
> //      size_t itemSize;  // total size of item (filesize, strlen  
> of symlink)
> //  };
> //
> // Some types need no further processing, such as directories; the
> //  higher level will know what to do with that immediately.
> // Other things (symlinks, files) will need more reading with
> //  readMoreData().
> //
> // The plugin is expected to skip to the next valid item whether the
> // current one was read or not, since the higher-level may choose to
> // filter a given item out of the installation for various reasons (a
> // symlink on windows? XML tags introduce filtering capabilities?)
> //
> // Returns 1 if there was another item, zero if EOF, -1 if error.  
> Errors
> // should be considered fatal to the installation, EOF should be the
> // normal loop termination condition.
> int getNextItem(void *handle, PluginNextItemInfo *nextItem);
>
> // Read more data from the current item...for symlinks, this will be
> // the name of the thing they point to, for files, this will be the
> // uncompressed contents of the item. Data is stored at buf, a buffer
> // of (*bufsize) bytes. On return, the number of bytes retrieved will
> // be stored to (*bufsize), whether there was a problem or not.
> //
> // Returns 1 if there was data to read, zero if EOF, -1 if error.  
> Errors
> // should be considered fatal to the installation, EOF should be the
> // normal loop termination condition. EOF only triggers when you  
> try to
> // read and there are no more bytes. A read that only fills one byte
> // before hitting the EOF will still return 1, and the next call will
> // return zero. This allows for archivers to return less than a full
> // buffer each call if it's more convenient to their decompression
> // algorithms.
> int readMoreData(void *handle, void *buf, size_t *bufsize);
>
>
> ...finally, clean up...
>
> void closeArchive(void *handle);
>
>
>
> ...you'll notice that the 1.0 plugin API has two methods: Size and  
> Copy.
>
> Size is intentionally removed...this was always less than useful  
> (as anyone that used it with a .tar.bz2 can tell you), and assumes  
> you want to install the whole archive, which may not be true in the  
> future if new XML tags show up to filter out items. Perhaps a  
> better solution to this is to assume the total size of an install  
> is 0 bytes unless explicitly stated using the existing XML  
> attributes, and if the install doesn't turn out to match the  
> expected size (0 or otherwise), pop up a message box saying so, and  
> reporting the actual result, so that the developer always fills  
> this in correctly during final testing before shipping the installer.
>
> The rest of the new API is, more or less, a broken-out version of  
> the Copy() method, but with the control put into the caller's hands  
> and expecting the plugin to do its work in smaller chunks, so the  
> caller can handle the general tasks of updating the UI and doing  
> the actual installing of files from one unified place.
>
> Comments?
>
> --ryan.
>
>
>
>

--
Stéphane Peter
megastep at megastep.org

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://icculus.org/pipermail/lokisetup/attachments/20050606/7f0b1ed8/attachment.htm>