Update of toolkit postbuild and project references strategy

Question

Update of toolkit postbuild and project references strategy

Closed this issue 4 years ago · 17 comments

As discussed benefits now to revisiting our approach to references to BHoM .dlls and management of toolkit responsibility for copying files to central user/program data location.

A few related steps to get agreement on, before potentially making changes across all Toolkits.
At point of making changes we should also add new required compliance check in CI for all toolkits (all that are not pre-alpha and are in installers at least)

Suggesting change location for installer and loading BHoM dlls to
C:\ProgramData\BHoM\Assemblies
This is naturally not unique to users, thus simpler to hardcode references to. Big advantage.
One thing to note. You do not need admin rights to write to ProgramData. Although other users will potentially need admin rights if using same machine to modify or uninstall BHoM once installed. Think this is fair - with a benefit to making BHoM version not user specific
Switch to referencing BHoM .dlls in Toolkits directly from the centralised ProgramData location (rather than relative Build folders as we are doing now)
Benefit here is huge as discussed - as user/developer can keep code references up to date via installers. Do not need to clone all dependencies and build just to work on derivative toolkit.
Devolve responsibility for copying files to ProgramData back to individual toolkits and repos.
i.e. BHoM_UI will no longer have a PostBuild.exe, blanket mining all conforming folders.
This is now possible to maintain easily, by including clear required CI check in all PRs.
One thing to consider carefully is the copying of Datasets across. These will logically sit in C:\ProgramData\BHoM\Datasets. We previously moved responsibility for this action to BHoM_UI (for good reason) Need to revisit.
Is it still BHoM_UI, or likely return to the Core BHoM_Dataset being responsible.
Similarly - worth taking this oppertunity to confirm best location for BHoMUpgraders.
Opportunity to promote to clear top level folder, such that we have:

C:\ProgramData\BHoM\Assemblies
    \Assemblies
    \Datasets
    \Upgrades

@FraserGreenroyd @adecler @IsakNaslundBh @rwemay
Some points still to discuss above there and get good agreement on approach. Thanks

Answer 1 · 2020-05-27T08:09:04.000Z

Looks very good to me. I built BHoM_UI yesterday for another PR/test, and this morning had another task to do with an unrelated bit of the code/tools that had broken - as is difficult to maintain all repo's sync'd. So two massive thumbs up from me!

Answer 2 · 2020-05-27T19:56:58.000Z

Capturing issues to address as discussed with @IsakNaslundBh and others this morning.

A. Locking dlls will be more common
&
B. Strategy for current postbuild event mining upgraders in BHoM_UI

A. Locking dlls will be more common.
By switching referencing all BHoM .dlls in Toolkits directly to the centralised ProgramData (i.e. building from same location as loading) overwriting .dlls already loaded in a UI will be prevented.

In practical terms this means Excel, Grasshopper etc. will need to be closed at the time of compiling any toolkit (not just at the time of compiling BHoM_UI as is currently the case)

Personally, and from discussions today, I think this is a fine compromise to facilitate the benefit of centralised .dlls.
Building individual repos with say Excel open will be possible - just the post build event overwriting .dll will fail. So simply rebuilding to checking compilation errors etc. with UI open for single toolkits (a use case mentioned) is possible.
Modifying chains of dependant toolkits (i.e. editing both BHoM_Engine and then GSA_Toolkit say) will not be possible. GSA_Toolkit will be pointing to locked .dll.

I think the loss of the fringe case is OK to facilitate the centralisation. But good to hear others thoughts as always?

Finally options to explore forced unloading of BHoM and .dlls from UIs. However challenges potentially with implementing consistently and so would not put this as critical for implementing this issues proposed changes.

B. Versioning_Toolkit and current postbuild event mining upgraders in BHoM_UI
Currently there are a series of actions handled by the BHoM_UI UI_PostBuild.exe that collect and aggregate together all of the Versioning_vX.json files.

In the immediate term I think we have a couple of options which will simply require developers to do one of two things to maintain and ensure their versioning is up to date.

i. Continue to rebuild the BHoM_UI last - triggering the the remaining CopyUpgrades parts of the current UI_PostBuild.exe

or

ii. frequently use the latest alpha installer to get all the up to date .dlls, datasets and versioning files.

N.B. As discussed, this second is something we will be wanting to encourage more and more for developers anyway, so users and coders are using same end product.

A separate question is should we migrate responsibility for the current versioning post builds to the Versioning_Toolkit? So developers doing versioning will need to call Versioning_Toolkit instead?

In medium term it will be useful to explore if the actual post build tasks of converting raw .json and aggregating could be done at run time for the current live milestone.
This would enable a similar fully decentralised approach to versioning - mush like Datasets where responsibility for moving to central location could be devolved to toolkits themselves as a very simple task.
@adecler good to get your thoughts on this point - prob warrants a bigger discussion in detail.

At end of milestone we can then still "lockdown" the aggregated .json into binary - so no longer editable and start afresh.

If we think the above medium term plan is a good idea - I might suggest we just leave responsibility of versioning migration to BHoM_UI for these final sprints of 3.2 while we sort the above non-versioning related tasks. To look at wider plans for next improvements to versioning in 3.3

Thoughts @IsakNaslundBh @adecler all

(P.S. sorry for long post 📃 )

TL;DR

A. Don't worry too much about locking .dlls 😄
B. To version as a developer - keep up to date with alpha installers (and/or just call BHoM_UI as before)

Answer 3 · 2020-05-28T09:33:19.000Z

All sounds good 👍 One more potential issue just came to my head: it is a good practice to stick to a single BHoM build/version per project, for obvious reasons. This may become problematic once the assemblies will be shared by many users or admin rights will be required.

Possibly it could be worth considering to allow multiple BHoM versions per machine? Manifest files (.gha, .addin, etc.) could sit in AppData per user then, pointing to the user's preferred version in ProgramData.

First blocker I see for such approach is Dynamo, where all files are copied over to its dedicated folder. However, as far as I remember, there should be a way to hack the .dll paths - maybe @adecler could say more?

Answer 4 · 2020-06-01T14:54:20.000Z

Thanks @pawelbaran - good point raised about having multiple parallel versions of BHoM installed simultaneously.

As discussed together - the use case of different versions of BHoM running simultaneously in different UI (say Excel vs GH vs Dynamo) - is actually prob one we want to explicitly avoid - to minimise unforeseen behaviour/user error.

With installers now relatively easy (and hopefully getting easier still!) to reinstall previous versions and upgrade/downgrade - suggesting we park parallel versions of BHoM for now.

A version of your proposed solution could be possible - but perhaps to be revisited if any future demand or user need is flagged. Hope makes sense?

Answer 5 · 2020-06-02T07:30:13.000Z

Agreed with OP. This looks like a great simplification of the current solution so always up for that 😄.

I would move versioning post build to the versioning toolkit. We're dealing with static json files here so build order is not an issue. There is just a new need to remember to build the versioning toolkit if the json files have been modified but I can live with that.

More and more I want to keep the location of the UI dlls a separate thing from the other dlls. Keeping UIs completely separate from each other is something that we want and even need to reduce issues of conflict and incorrectly exposing things from other UIs. This obviously doesn't mean that we have to copy all dlls to each UI folder like we do now. we could better leverage the PATH system variable for that.

Answer 6 · 2020-06-02T13:21:54.000Z

Following this morning discussion - the following plan has been agreed for actioning this issue.

Pre-work

to be completed in advance of 13:00 UTC Wed 03 June

(@awakeman) Update installer
a. Change target directory to ProgramData
b. Add new step to remove AppData BHoM folders - to clean out for old installations.
For pre-work - agreed assumed .dlls are already in correct location. Ready for Thursday's work for changing build location etc.
(@IsakNaslundBh) Update Library_Engine to point new location
(@IsakNaslundBh) Update BHoMFolder query
https://github.com/BHoM/BHoM_Engine/blob/30240fdfe964255232013497ea86fcba48ce65a2/Reflection_Engine/Query/BHoMFolder.cs
(@IsakNaslundBh) Update default reference in File adapter
(@adecler ) Update Versioning to accommodate changes as discussed.
Merging with files from ProgramData.
Further issues to be raised for more improvements in next milestone.
(@adecler ) Update Parent BHoM_UI
Agreed BHoM_UI post build will still be used for managing versioning. (minimise change now as further improvements will follow)
Also noted Dynamo relies on post-build so will also be handled as switch as needed.

Update of individual UIs

Update of compliance checks to assist in upcoming PRs, as well as beyond

(@FraserGreenroyd) Update Test Toolkit for compliance check
Check added to existing compliance checks
Discrete Required system check will be added later as a further improvement

Workshop actions

to be completed from 08:30 UTC Thursday 03 June onwards

Test installer created prior to workshop
Update all toolkits - prioritising those in installers for this first wave:
A) all .proj should own their own post build events
B) all .proj should reference dependencies via ProgramData
C) Agreed (as before) projects in common solutions should continue to reference each other directly - not via .dll
@IsakNaslundBh creating/created script to assist with references for 90% of base case.
Add additional exceptional step to post build for BHoM core.dll to check folder exists.
All others to be simple copy into location - will not need to check also as reference BHoM - so folder will have to already exist to build. (either from installer or manual build of BHoM Repo.
deploy and test new installer via azure

Post workshop and completion of this issue

New issues to be raised specifically for

Further improvements and full decentralisation of Versioning
Refactoring of referencing for UIs. That is for example in the case of Grasshopper. The Grasshopper_Toolkit .dlls should not be in the central ProgramData folder. Will minimise conflicts between UIs.
Update for Msbuild_Toolkit and all other toolkits that are not in installers
Review our use of the common Build folder for .projs now following this change. Any benefit in changing. To be discussed.
Updating documentation

Answer 7 · 2020-06-02T13:22:36.000Z

Agreed branch name for this work is

BHoM_Admin-#5-Referencing

Answer 8 · 2020-06-02T13:23:57.000Z

@adecler @FraserGreenroyd @IsakNaslundBh @awakeman @pawelbaran please see notes above to action plan as agreed this morning.

Answer 9 · 2020-06-03T07:07:44.000Z

For those relying on UI_PostBuild, please not that it now uses two new optional arguments:
- --onlyUpgrades: use this to make sure only upgrade files are created (no dll or dataset)
- --cleanTargetDir: use this to have the target folder cleared first
@IsakNaslundBh , I kept the target folder to Assemblies/bin to mirror what you did in the engine (ToNewVersion file) but it is actually very easy to use the BHoM/Upgrades folder instead as nothing changes in the code, only in the post build command. So happy to do the switch now.
Dynamo will continue doing its own thing for now. There is actually no changes needed due to the modifications on UI_PostBuild

Answer 10 · 2020-06-03T16:58:34.000Z

@IsakNaslundBh , I kept the target folder to Assemblies/bin to mirror what you did in the engine (ToNewVersion file) but it is actually very easy to use the BHoM/Upgrades folder instead as nothing changes in the code, only in the post build command. So happy to do the switch now.

Have updated BHoM/BHoM_Engine#1823 to your suggestion here @adecler . Did change the folder to Upgrades without the bin.

Answer 11 · 2020-06-03T17:34:19.000Z

On post-builds
As agreed the responsibility for moving .dll is now that of the .csproj
Specifically the project .dll itself $(TargetFileName)

But also any of its dependencies - NuGet acquired dlls or similar MUST also be copied over to C:\ProgramData\BHoM\Assemblies as an additional explicit line in the post build event.

standard default post-build command:
xcopy "$(TargetDir)$(TargetFileName)" "C:\ProgramData\BHoM\Assemblies" /Y

Additional creation of system folders post-build events will be added to BHoM.csproj only

mkdir "C:\ProgramData\BHoM\Assemblies"
mkdir "C:\ProgramData\BHoM\Datasets"
mkdir "C:\ProgramData\BHoM\Upgrades"

to remind - this is because BHoM.dll is the primary dependency and thus is guaranteed to be built (or will come with installer). Thus no other toolkit should have to worry about creation of folders (only copying into them)

Answer 12 · 2020-06-04T03:35:32.000Z

Have updated BHoM/BHoM_Engine#1823 to your suggestion here @adecler . Did change the folder to Upgrades without the bin.

Thanks @IsakNaslundBh . I have updated Prs on both BHoM_UI and Versioning_Toolkit to reflect that

Answer 13 · 2020-06-04T07:40:36.000Z

standard default post-build command:
copy "$(TargetDir)$(TargetFileName)" "C:\ProgramData\BHoM\Assemblies" /Y

@al-fisher this differs slightly from what I had put in place for the automated script:

xcopy "$(TargetPath)" "C:\ProgramData\BHoM\Assemblies" /C /Y

Remember having some issue with copy at one point, but cant remember exactly what it was and why. Trying to figure out the difference now, but should agree on something before actioning all the changes today!

@adecler @FraserGreenroyd

Answer 14 · 2020-06-04T07:48:09.000Z

I used xcopy in Versioning toolkit because some of the options (like auto-creation of folders) make it easier and more concise.

Answer 15 · 2020-06-04T08:33:33.000Z

As agreed - I have updated the standard default post-build command see above to:
xcopy "$(TargetDir)$(TargetFileName)" "C:\ProgramData\BHoM\Assemblies" /C /Y

Agreement to standardise around xcopy to help with extending for exceptional cases ~~and including /C switch also for robustness of process.~~

Also agreed "$(TargetDir)$(TargetFileName)" separation of the two variables (rather than using the similar targetpath) is valuable for transparency.

Answer 16 · 2020-06-04T11:08:10.000Z

Agreed removal of the /C switch to give louder error - rather than silent sharing violation.

Is updated above.

Answer 17 · 2020-06-04T17:25:05.000Z

Pre-work and workshop actions as listed above in comment #5 (comment) have successfully been completed with all toolkits included in installers updated to reflect the required changes.

Post workshop actions still required naturally including further testing of alphas beyond that carried out today.
Further issues to be raised for post-workshop actions.

Thanks @FraserGreenroyd @adecler @IsakNaslundBh @pawelbaran @awakeman @kThorsager @MajaLindroth @peterjamesnugent in particular for efforts today 😄 🎉