threerings / getdown

Download, Install, Update
Other
507 stars 125 forks source link

Add glob 'cleanup_pattern' to remove obsolete resources #253

Closed gieza closed 3 years ago

gieza commented 3 years ago

Hello,

We noticed that over time obsolete resources accumulate in Getdown folder. This PR allows to define optional list of Glob patterns in Getdown.txt, like:

cleanup_pattern = lib\*.jar
cleanup_pattern = rsrcs\*.jpg
cleanup_pattern = old_*.png

Glob patterns are defined relative to 'getdown' folder.
In this case Getdown would collect list for files from the patterns, that is cross-check against resources specified in Getdown.txt. Then only the files that are not in getdown resource list, will be removed.

Potentially closes #183

samskivert commented 3 years ago

This is great thanks! I'll try to review it today or tomorrow and either give feedback or merge it. Thanks again!

samskivert commented 3 years ago

Looks great, thanks again!

gieza commented 3 years ago

Hi,

you are welcome, though there's not much to thank for. We needed ourselves and pull request did not cause a lot of extra work.

I am working on several modifications of Getdown as per list below, that I will push to my fork. If you think those are interesting, I can create pull requests as well.

samskivert commented 3 years ago

That all sounds great. I'm sure everyone will appreciate those improvements.

serge-xav commented 2 years ago

Hi all, unfortunately, with latest commit (332f7f1) , the feature works only for Versioned updates. Unversioned updates will never clean the resource. I'd rather have the cleanup possible for both Versioned and Unversioned, but as soon as there was some update on the getdown.txt file. What about moving the cleanup code at the end of install() method, if there is updateAvailable ?

bekoenig commented 2 years ago

Hej @serge-xav,

you are right. I moved this snip

     /**
     * Installs the currently pending new resources.
     */
    public void install () throws IOException
    {
        if (SysProps.noInstall()) {
            log.info("Skipping install due to 'no_install' sysprop.");
        } else if (isUpdateAvailable()) {
            log.info("Installing " + _toInstallResources.size() + " downloaded resources:");
            for (Resource resource : _toInstallResources) {
                resource.install(true);
            }
            _toInstallResources.clear();
            _readyToInstall = false;
            log.info("Install completed.");

            // do resource cleanup
            final List<String> cleanupPatterns = _app.cleanupPatterns();
            if (!cleanupPatterns.isEmpty()) {
                cleanupResources(cleanupPatterns);
            }
        } else {
            log.info("Nothing to install.");
        }
    }

and I think this should be better!

Greetz, Ben

serge-xav commented 2 years ago

Hi, thanks for your commit, I'll test it on my side but it should be OK from my point of view.

BR Xavier