wee_extension --uninstall does not remove multiple SKIN_ROOT skin directories

[Pat O'Brien]

I'm working on an update to my Belchertown skin which will really add 2 skins to SKIN_ROOT. One skin is the HTML files, and the other is a highcharts fork. Certain users are having a high latency running the skin as it is today, so splitting into 2 offers a faster generation time since highcharts has a lot database queries, and a separate skin offers less SLE loops.  

During my testing I found that the --uninstall function does not remove multiple SKIN_ROOT directories that the install.py placed there. 

I found in weecfg/extensions.py, the uninstaller command is only removing those directories with the most common root - which does not apply to all SKIN_ROOT directories since there are multiple roots. 

I made a small change to the extensions.py (diff below) which handles the multiple root directory removal. Instead of finding the commonprefix, I just loop through the directory_list, then loop through each item for removal. 

root@setuppy:~# diff extension.py extension_updated.py

402,407c402,407

<         # Start by finding the directory closest to root

<         most_root = os.path.commonprefix(directory_list)

<         # Now delete the directories under it, from the bottom up.

<         for dirpath, _, _ in os.walk(most_root, topdown=False):

<             if dirpath in directory_list:

<                 self.delete_directory(dirpath)

---

>         for dir in directory_list:

>             # Loop through each directory listing

>             for dirpath, _, _ in os.walk(dir, topdown=False):

>                 # Delete the directory

>                 if dirpath in directory_list:

>                     self.delete_directory(dirpath)

I wanted to raise it here to see if this is a valid change and discuss. Happy to create an issue and PR if so. 

[gjr80]

Interesting, WeeWX-WD installs 4 skins and I have never had a case where --uninstall has not removed them all. Pushed for time this morning but I will sit down and have a play with this this arvo. Pat, I presume the just use your repo as per the latest commit?

Gary

[Pat O'Brien]

Not yet, I have yet to commit my most recent changes as I wanted to do a few more tests. 

I forgot to mention that I noticed this error on a vanilla 3.8.2 setup.py install method. I'll try out a packaged version and commit my changes and will let you know. 

[Pat O'Brien]

I've actually run out of time for the night - so no rush on getting back to me. Here's my output to reproduce this. 

My GitHub repo master branch is up to date now with what will become version 0.7 after a bit more testing. 

Simple wget of the master branch

root@setuppy:~# wget https://github.com/poblabs/weewx-belchertown/archive/master.tar.gz
2018-09-05 23:46:36 (1016 KB/s) - ‘master.tar.gz’ saved [100497]

This test is on a setup.py installation, so /home/weewx/bin/wee_extension. Install the extension with high verbosity

root@setuppy:~# /home/weewx/bin/wee_extension --install master.tar.gz --verbosity 3
Request to install 'master.tar.gz'
Extracting from tar archive master.tar.gz
  Request to install extension found in directory /var/tmp/weewx-belchertown-master
  Found extension with name 'Belchertown'
  Copying new files
    Copying from '/var/tmp/weewx-belchertown-master/bin/user/belchertown.py' to '/home/weewx/bin/user/belchertown.py'
    Copying from '/var/tmp/weewx-belchertown-master/bin/user/belchertown_highchartsSearchX.py' to '/home/weewx/bin/user/belchertown_highchartsSearchX.py'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/favicon.ico' to '/home/weewx/skins/Belchertown/favicon.ico'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/footer.html.tmpl' to '/home/weewx/skins/Belchertown/footer.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/header.html.tmpl' to '/home/weewx/skins/Belchertown/header.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/index.html.tmpl' to '/home/weewx/skins/Belchertown/index.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/about.inc.example' to '/home/weewx/skins/Belchertown/about.inc.example'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/records.inc.example' to '/home/weewx/skins/Belchertown/records.inc.example'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/skin.conf' to '/home/weewx/skins/Belchertown/skin.conf'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/style.css' to '/home/weewx/skins/Belchertown/style.css'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/about/index.html.tmpl' to '/home/weewx/skins/Belchertown/about/index.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/graphs/index.html.tmpl' to '/home/weewx/skins/Belchertown/graphs/index.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/NOAA/NOAA-YYYY-MM.txt.tmpl' to '/home/weewx/skins/Belchertown/NOAA/NOAA-YYYY-MM.txt.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/NOAA/NOAA-YYYY.txt.tmpl' to '/home/weewx/skins/Belchertown/NOAA/NOAA-YYYY.txt.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/records/index.html.tmpl' to '/home/weewx/skins/Belchertown/records/index.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/reports/index.html.tmpl' to '/home/weewx/skins/Belchertown/reports/index.html.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/highcharts-dayplots.js.tmpl' to '/home/weewx/skins/Belchertown/js/highcharts-dayplots.js.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/highcharts-weekplots.js.tmpl' to '/home/weewx/skins/Belchertown/js/highcharts-weekplots.js.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/highcharts-monthplots.js.tmpl' to '/home/weewx/skins/Belchertown/js/highcharts-monthplots.js.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/highcharts-yearplots.js.tmpl' to '/home/weewx/skins/Belchertown/js/highcharts-yearplots.js.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/index.html' to '/home/weewx/skins/Belchertown/js/index.html'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/js/responsive-menu.js' to '/home/weewx/skins/Belchertown/js/responsive-menu.js'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/json/index.html' to '/home/weewx/skins/Belchertown/json/index.html'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/json/weewx_data.json.tmpl' to '/home/weewx/skins/Belchertown/json/weewx_data.json.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/clear-day.png' to '/home/weewx/skins/Belchertown/images/clear-day.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/clear-night.png' to '/home/weewx/skins/Belchertown/images/clear-night.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/cloudy.png' to '/home/weewx/skins/Belchertown/images/cloudy.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/fog.png' to '/home/weewx/skins/Belchertown/images/fog.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/hail.png' to '/home/weewx/skins/Belchertown/images/hail.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/partly-cloudy-day.png' to '/home/weewx/skins/Belchertown/images/partly-cloudy-day.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/partly-cloudy-night.png' to '/home/weewx/skins/Belchertown/images/partly-cloudy-night.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/rain.png' to '/home/weewx/skins/Belchertown/images/rain.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/sleet.png' to '/home/weewx/skins/Belchertown/images/sleet.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/snow.png' to '/home/weewx/skins/Belchertown/images/snow.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/snowflake-icon-15px.png' to '/home/weewx/skins/Belchertown/images/snowflake-icon-15px.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/sunrise.png' to '/home/weewx/skins/Belchertown/images/sunrise.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/sunset.png' to '/home/weewx/skins/Belchertown/images/sunset.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/thunderstorm.png' to '/home/weewx/skins/Belchertown/images/thunderstorm.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/tornado.png' to '/home/weewx/skins/Belchertown/images/tornado.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/wind.png' to '/home/weewx/skins/Belchertown/images/wind.png'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown/images/index.html' to '/home/weewx/skins/Belchertown/images/index.html'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown_Highcharts/skin.conf' to '/home/weewx/skins/Belchertown_Highcharts/skin.conf'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown_Highcharts/json/day.json.tmpl' to '/home/weewx/skins/Belchertown_Highcharts/json/day.json.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown_Highcharts/json/week.json.tmpl' to '/home/weewx/skins/Belchertown_Highcharts/json/week.json.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown_Highcharts/json/month.json.tmpl' to '/home/weewx/skins/Belchertown_Highcharts/json/month.json.tmpl'
    Copying from '/var/tmp/weewx-belchertown-master/skins/Belchertown_Highcharts/json/year.json.tmpl' to '/home/weewx/skins/Belchertown_Highcharts/json/year.json.tmpl'
  Copied 46 files
  Adding sections to configuration file
    Merged extension settings into configuration file
  Adding services to service lists
Saving installer file to /home/weewx/bin/user/installer/Belchertown
Saved configuration dictionary. Backup copy at /home/weewx/weewx.conf.20180905234706
Finished installing extension 'master.tar.gz'

So far so good. No frills yet. 

New skin folders installed (bold and underline for standout)

root@setuppy:~# ls -al /home/weewx/skins/
total 28
drwxr-xr-x  7 root root 4096 Sep  5 23:47 .
drwxr-xr-x  9 root root 4096 Sep  5 23:47 ..
drwxr-xr-x 10 root root 4096 Sep  5 23:47 Belchertown
drwxr-xr-x  3 root root 4096 Sep  5 23:47 Belchertown_Highcharts
drwxr-xr-x  2 root root 4096 Sep  5 16:47 Ftp
drwxr-xr-x  2 root root 4096 Sep  5 16:47 Rsync
drwxr-xr-x  6 root root 4096 Sep  5 16:47 Standard

Ok, time to uninstall. High verbosity. 

root@setuppy:~# /home/weewx/bin/wee_extension --uninstall Belchertown --verbosity 3
Request to remove extension 'Belchertown'
  Removing files.
  Deleting file /home/weewx/bin/user/belchertown.py
  Deleting file /home/weewx/bin/user/belchertown.pyc
  Deleting file /home/weewx/bin/user/belchertown.pyo
  Deleting file /home/weewx/bin/user/belchertown_highchartsSearchX.py
  Deleting file /home/weewx/bin/user/belchertown_highchartsSearchX.pyc
  Deleting file /home/weewx/bin/user/belchertown_highchartsSearchX.pyo
  Deleting file /home/weewx/skins/Belchertown/favicon.ico
  Deleting file /home/weewx/skins/Belchertown/footer.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/header.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/index.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/about.inc.example
  Deleting file /home/weewx/skins/Belchertown/records.inc.example
  Deleting file /home/weewx/skins/Belchertown/skin.conf
  Deleting file /home/weewx/skins/Belchertown/style.css
  Deleting file /home/weewx/skins/Belchertown/about/index.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/graphs/index.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/NOAA/NOAA-YYYY-MM.txt.tmpl
  Deleting file /home/weewx/skins/Belchertown/NOAA/NOAA-YYYY.txt.tmpl
  Deleting file /home/weewx/skins/Belchertown/records/index.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/reports/index.html.tmpl
  Deleting file /home/weewx/skins/Belchertown/js/highcharts-dayplots.js.tmpl
  Deleting file /home/weewx/skins/Belchertown/js/highcharts-weekplots.js.tmpl
  Deleting file /home/weewx/skins/Belchertown/js/highcharts-monthplots.js.tmpl
  Deleting file /home/weewx/skins/Belchertown/js/highcharts-yearplots.js.tmpl
  Deleting file /home/weewx/skins/Belchertown/js/index.html
  Deleting file /home/weewx/skins/Belchertown/js/responsive-menu.js
  Deleting file /home/weewx/skins/Belchertown/json/index.html
  Deleting file /home/weewx/skins/Belchertown/json/weewx_data.json.tmpl
  Deleting file /home/weewx/skins/Belchertown/images/clear-day.png
  Deleting file /home/weewx/skins/Belchertown/images/clear-night.png
  Deleting file /home/weewx/skins/Belchertown/images/cloudy.png
  Deleting file /home/weewx/skins/Belchertown/images/fog.png
  Deleting file /home/weewx/skins/Belchertown/images/hail.png
  Deleting file /home/weewx/skins/Belchertown/images/partly-cloudy-day.png
  Deleting file /home/weewx/skins/Belchertown/images/partly-cloudy-night.png
  Deleting file /home/weewx/skins/Belchertown/images/rain.png
  Deleting file /home/weewx/skins/Belchertown/images/sleet.png
  Deleting file /home/weewx/skins/Belchertown/images/snow.png
  Deleting file /home/weewx/skins/Belchertown/images/snowflake-icon-15px.png
  Deleting file /home/weewx/skins/Belchertown/images/sunrise.png
  Deleting file /home/weewx/skins/Belchertown/images/sunset.png
  Deleting file /home/weewx/skins/Belchertown/images/thunderstorm.png
  Deleting file /home/weewx/skins/Belchertown/images/tornado.png
  Deleting file /home/weewx/skins/Belchertown/images/wind.png
  Deleting file /home/weewx/skins/Belchertown/images/index.html
  Deleting file /home/weewx/skins/Belchertown_Highcharts/skin.conf
  Deleting file /home/weewx/skins/Belchertown_Highcharts/json/day.json.tmpl
  Deleting file /home/weewx/skins/Belchertown_Highcharts/json/week.json.tmpl
  Deleting file /home/weewx/skins/Belchertown_Highcharts/json/month.json.tmpl
  Deleting file /home/weewx/skins/Belchertown_Highcharts/json/year.json.tmpl
  Removed 46 files
  Deleting directory /home/weewx/skins/Belchertown/json
  Deleting directory /home/weewx/skins/Belchertown/records
  Deleting directory /home/weewx/skins/Belchertown/images
  Deleting directory /home/weewx/skins/Belchertown/NOAA
  Deleting directory /home/weewx/skins/Belchertown/graphs
  Deleting directory /home/weewx/skins/Belchertown/js
  Deleting directory /home/weewx/skins/Belchertown/reports
  Deleting directory /home/weewx/skins/Belchertown/about
  Deleting directory /home/weewx/skins/Belchertown
Finished removing extension 'Belchertown'

Notice it has removed the Belchertown_Highcharts files, but did not remove the Belchertown_Highcharts directory itself. Notice the directories it's removed has omitted "/home/weewx/skins/Belchertown_Highcharts". 

And a listing to show the folder remains. bold and underline for standout. 

root@setuppy:~# ls -al /home/weewx/skins/
total 24
drwxr-xr-x 6 root root 4096 Sep  5 23:48 .
drwxr-xr-x 9 root root 4096 Sep  5 23:48 ..
drwxr-xr-x 3 root root 4096 Sep  5 23:48 Belchertown_Highcharts
drwxr-xr-x 2 root root 4096 Sep  5 16:47 Ftp
drwxr-xr-x 2 root root 4096 Sep  5 16:47 Rsync
drwxr-xr-x 6 root root 4096 Sep  5 16:47 Standard

If you try my diff above, it'll remove the Belchertown_Highcharts directory. What I am unsure of is if this has any other implications across weewx elsewhere. Also, as mentioned, haven't had time to test on a packaged version (apt) of weewx yet. 

Thanks

[gjr80]

Yep, did the same for me. Wanted to understand why WeeWX-WD never had the problem, first thought was what user was running wee_extension, dead end. Then I wondered if the json sub-directory was doing it. I had a quick look at the extension.py code, I hadn't come across os.path.commonprefix() before so googled it. os.path.commonprefix() checks a list of paths and returns the longest common prefix and it does this by doing a character-by-character comparison, when I saw the warning that it could return invalid paths that peaked my curiosity. In the case of your extension /home/weewx/skins/Belchertown is the result, so extension.py then works from /home/weewx/skins/Belchertown down and thus /home/weewx/skins/Belchertown_Highcharts is ignored. I did some rejigging of your extension so that it used /home/weewx/skins/ZBelchertown_Highcharts instead and it worked fine. That explains why WeeWX-WD was never affected, all of its skin directory names had no common prefix. Probably more correctly (and I didn't check this) but the fact that the common prefix includes the name of one of the skin directories (Belchertown) may be the cause.

No matter, its a bug that needs to be fixed. Can't comment on the proposed fix, better for Tom or Matthew I think.

Gary

[Thomas Keffer]

That sounds like a reasonable analysis. 

This is an obscure problem. Can we just use a workaround and call the two skins "Belchertown" and "Highcharts_Belchertown," or something similar? 

BTW, Pat, how are you enjoying your new life as a maintainer of open source code? :-)

-tk

[Pat O'Brien]

Tom, 

It's fun and I enjoy the challenges. I've got a few other open source projects out there but with WordPress plugins published in their directory. The problem is there's not enough time in the day to maintain them all!

I can give it a try to rename the skin directories as you suggest and give it a test. 

Thanks for checking it out. 

[Pat O'Brien]

Confirmed that this works when renaming the directory to "Highcharts_Belchertown" - while it's not as pretty as having the directories next to each other in a listing - it gets the job done for an install/uninstall perspective. 

Thanks for the suggestion. 

[Vince Skahan]

On Thursday, September 6, 2018 at 5:40:42 AM UTC-7, Tom Keffer wrote:

BTW, Pat, how are you enjoying your new life as a maintainer of open source code? :-)

....and Pat was never heard from again... 

[Pat O'Brien]

hah! 

Tom, I see I can edit the weewx wiki on GitHub. Is it OK to edit and add the link to the Belchertown skin - or would you rather control that update?

[Thomas Keffer]

It's a Wiki! Anyone can edit. Go at it!

[Pat O'Brien]

Will do, thanks!

Another question which is sort of related. Is it possible for Skin A to get the [[[Extras]]] settings of Skin B from within an SLE?

Instead of maintaining 2 sets of Extras for 2 skins, it'd be nice to have 1 across both skins. 

I started to play around with something like this, but it doesn't catch any skin.conf settings. So before I got too deep, I figured there's probably an easier way?

self.generator.config_dict["StdReport"]["Belchertown"]["Extras"]

[gjr80]

Pat,

Through an SLE you can access anything within the WeeWX universe, you just need to know where to find it and the (python) code to access it.

So what does self.generator.config_dict give you? On a plain vanilla WeeWX install I can see then entire weewx.conf.

Gary

[Pat O'Brien]

Hi Gary, I understand that part (hopefully displayed through the bit of code I shared) - what my concern was that if I'm getting the [[[Extras]]] from the config directly, that doesn't account for anything in skin.conf. 

So I was asking if there's a way to get "The Extras" (as merged by both config file's Extras)...

[gjr80]

Ok, Sorry, misunderstood. No, you will need to go through the process like class StdReportEngine does in reportengine.py to get the skin name, work out its location and then read the config from its skin.conf.

Gary

[Pat O'Brien]

No worries. I think from this section of reportengine.py, I've found what I'm looking for. 

Get the skin.conf os path and create a ConfigObj. Then merge that ConfigObj with the skin settings from weewx.conf.