Fix copystep when run via gpup #55
Conversation
The CopyStep serialisation was using the wrong destination path for recursive directories, and using the wrong flag for validate.
|
@chcg did you say you'd left a comment here? Responding to the comment from the other issue... yes, it would be great to have unit tests (and ideally some integration tests) for all of the installation steps, but there's currently nothing in place for it. We'll need to think about mocking the filesystem, or injecting something to handle the file calls. I'd like to get this out, and attack unit tests as a separate item - this is a pretty ugly bug. |
| if (_validate) { | ||
| copyElement->SetAttribute(_T("recursive"), _T("true")); | ||
| copyElement->SetAttribute(_T("validate"), _T("true")); |
There was a problem hiding this comment.
Just for my understanding, was this issue introduced with
Revision: 91f929d
Author: Dave Brotherstone [email protected]
Date: 08.10.2016 10:29:02
Message:
Set variables for GPUPCreate a psuedo action for GPUP to set all the variables, allows passing configuration (such as the validation URL for the copy step)
Modified: gpup/src/gpup.cpp
Modified: libinstall/include/libinstall/VariableHandler.h
Modified: libinstall/src/CopyStep.cpp
Modified: libinstall/src/InstallStepFactory.cpp
Modified: libinstall/src/VariableHandler.cpp
Modified: pluginManager/src/PluginList.cpp
or never working at all? Thanks for stepping in to this.
There was a problem hiding this comment.
Missed to hit submit, so it was not visible.
There was a problem hiding this comment.
Yeah, that was it. I recognised it as soon as I saw what was happening in the PluginManagerGpup.xml file.
There was a problem hiding this comment.
Otherwise the fix looks ok to me.
|
Do you think we can do a release now? |
|
Yes, i think this should go out. I wanted to update the version and run some tests on upgrading tonight, then push the list to both locations. I can't do anything more until tonight, but if you can do the version update, that'd be great (no worries if not, I can do it tonight). |
|
I will do it the version update now. Would else should I do on vacation go out into the sun and get burned ;-) |
|
Whaaa... haha, no leave it, enjoy your vacation! I'll sort it tonight :) |
|
Already tagged it and created the release, see https://github.com/bruderstein/nppPluginManager/releases/edit/v1.4.5. Already added some description before appveyor was ready, but that is gone now :-( Also added 1.4.5 to the PM list, but don't get it via devlist. |
|
Devlist seems to be working now (i just ticked it and got the 1.4.5 version - it can take a few minutes for the changes to be applied. Release looks great, thanks. I've run through a clean install of N++ 7.3.2, updated to 1.4.3, then updated to 1.4.5 and it went smoothly. I'll update the old list so it shows the update directly to Plugin Manager 1.4.5, then after a few days I'll update it so that it only shows the new Plugin Manager, forcing people to update rather than stay on the old version (I don't want to keep maintaining the old list location, although I guess we'll have to for a while) |
|
Done - all updated. It'll take 24 hours or so for the caches to update, so not everyone sees it immediately. I've also update the list for 1.3.5 (and earlier), so they'll be updated to 1.4.5 directly too. @chcg Thanks again for your help, enjoy your vacation! |
The CopyStep serialisation was using the wrong destination path for recursive directories, and using the wrong flag for validate.
Fixes #52