Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revision of build process part 1 #3137

Merged
merged 43 commits into from
Oct 25, 2019
Merged

Revision of build process part 1 #3137

merged 43 commits into from
Oct 25, 2019

Conversation

donker
Copy link
Contributor

@donker donker commented Oct 10, 2019

This is the first in a series of revisions of the way we build DNN. It follows the merging on AE into the main repository. What this merge does is the following:

  1. Move all content from ./Website to ./DNN Platform/Website and merge with what was there. The goal is to make it clear what is under source control and what is the website being built. ./DNN Platform/Website is source. ./Website is where the build process copies content to so it can start packaging.

  2. Move the DNN packaging from MSBuild to Cake. Cake is better suited for this type of task and the old MSBuild tasks were very convoluted due to legacy code.

@donker donker requested a review from valadas October 10, 2019 09:39
@mitchelsellers
Copy link
Contributor

@donker I'm not sure what the issue is here, but this showing lots of conflicts that restrict the merge?

@mitchelsellers
Copy link
Contributor

\azp run

bdukes
bdukes previously approved these changes Oct 10, 2019
.gitignore Outdated Show resolved Hide resolved
Build/Cake/thirdparty.json Outdated Show resolved Hide resolved
@bdukes
Copy link
Contributor

bdukes commented Oct 10, 2019

I've rebased on release/9.4.x, no more merge conflicts

bdukes
bdukes previously approved these changes Oct 10, 2019
bdukes
bdukes previously approved these changes Oct 10, 2019
SolutionInfo.cs Outdated
Comment on lines 37 to 39
[assembly: AssemblyVersion("9.4.2.0")]
[assembly: AssemblyFileVersion("9.4.2.5782")]
[assembly: AssemblyInformationalVersion("9.4.2-unstable.5782+Branch.reorg1.Sha.b37c5e45ed0dde233bee7c8572d70940e5bda93e")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be added in here? Or when the manifests are reverted, should this be reverted as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll repair after merge in a fresh PR.

bdukes
bdukes previously approved these changes Oct 10, 2019
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so from a read of the changes, I have nothing to say, this is a major improvement!

However I tried to install a clean site and the Persona Bar does not load because of this missing file
image

I would like that we get to a state that works before merging. I need to catch a few zzz, but I will ping you up in the afternoon...

@donker
Copy link
Contributor Author

donker commented Oct 11, 2019

I would like that we get to a state that works before merging. I need to catch a few zzz, but I will ping you up in the afternoon...

I do not get the same result here. Strange.

@valadas
Copy link
Contributor

valadas commented Oct 11, 2019

I will try to give it a more detailed look in a few hours, sorry.

@david-poindexter
Copy link
Contributor

I have tested this locally and am seeing the same as @valadas

@valadas
Copy link
Contributor

valadas commented Oct 11, 2019

Ok, one difference here needs to be fixed:
image
This should be named .resources

@valadas
Copy link
Contributor

valadas commented Oct 11, 2019

image

Here is one finding, the resources.zip on Dnn.PersonaBar.Extensions is missing the output from the yarn build and has other files unrelated, maybe this is getting zipped from the wrong source path or something?

@donker
Copy link
Contributor Author

donker commented Oct 14, 2019

Here is one finding, the resources.zip on Dnn.PersonaBar.Extensions is missing the output from the yarn build and has other files unrelated, maybe this is getting zipped from the wrong source path or something?

I can't replicate that, Daniel. Here it builds this resource zip correctly from Yarn. Can you check your build output?

@donker
Copy link
Contributor Author

donker commented Oct 14, 2019

Ok, one difference here needs to be fixed:
image
This should be named .resources

I just fixed this.

@valadas
Copy link
Contributor

valadas commented Oct 14, 2019

@donker well, I see the yarn build running but then the packaging does not have the build result, I am thinking maybe the build script has the wrong source folder. David had the same results as me. Can you try re-cloning into a brand new folder and building to see, maybe you have some ignored files from past builds that makes it work for you but not for us...

I am still a bit swamped today but if you want to take a look at it together I should have a bit more time starting tomorrow... Just ping me up.

@donker
Copy link
Contributor Author

donker commented Oct 15, 2019

It is probably due to the various paths in the two MS build solutions running in parallel. It's a royal clusterf. We could try to solve this as well by collapsing those two into a single one. But it requires quite a bit of work to figure out all the dependencies. I just gave it a shot but no cigar yet.

I propose to move AE's module.build to a file called AEModule.build and similar with Package.targets. These are the two files that are different from the main files and specifically tweaked for the AE.

@bdukes bdukes mentioned this pull request Oct 15, 2019
3 tasks
@valadas
Copy link
Contributor

valadas commented Oct 15, 2019

Cool, well ping me up if you want an extra pair of eyes, I got it working before this PR by referencing the main build script for everything but having 1 override for the AE Extensions project which was a bit different.

@bdukes
Copy link
Contributor

bdukes commented Oct 15, 2019

I can confirm that a clean install results in /Install/Module/Dnn.PersonaBar.UI_09.02.02.Install.zip with a Resources.zip which is missing its /scripts/exports folder. When re-running the build a second time, that folder is in place.

It seems like there may be timing issues around when things are built/packaged/cleaned.

@valadas
Copy link
Contributor

valadas commented Oct 20, 2019

Ok, so just updated with the changes that fixed previous build issue and it did not solve this one, it looks like we have missing stuff in PersonaBar.UI and EditBar.UI packages for some reason.

@donker
Copy link
Contributor Author

donker commented Oct 20, 2019

I went ahead and merged the build scripts from DNN and AE into one folder. Hopefully this resolves the quirks you were seeing.

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

I've rebased this PR on release/9.4.x again.

Once it's done building again, we can check the artifacts produced in Azure DevOps and see if they're still missing folders.

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

The new build's artifacts are still missing /Install/Module/Dnn.PersonaBar.UI.09.04.02_Install.zip/Resources.zip/scripts/exports/export-bundle.js (otherwise, it appears to be complete).

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

That Resources.zip is created on line 7113 of the build output, and the lerna build command which creates the export-bundle finishes on line 8998

The Resources.zip is created in the Minification task:

<Target Name="Minification">
<MakeDir Directories="$(MSBuildProjectDirectory)\Package"/>
<Zip Files="@(Resources)" WorkingDirectory="$(MSBuildProjectDirectory)" ZipFileName="$(MSBuildProjectDirectory)\Package\Resources.zip" />

The Minification task is run by the Package task:

<Target Name="Package" Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU'">
<XmlRead Prefix="n"
Namespace="http://schemas.microsoft.com/developer/msbuild/2003"
XPath="dotnetnuke/packages/package/@version"
XmlFileName="$(DNNFileName).dnn">
<Output TaskParameter="Value" PropertyName="Version" />
</XmlRead>
<ExtensionPackager Manifest="$(DNNFileName).dnn" WorkingFolder="$(MSBuildProjectDirectory)\">
<Output TaskParameter="PackageFiles" PropertyName="PackageFiles">
</Output>
</ExtensionPackager>
<CallTarget Targets="Minification"/>

The Package task is in a list of tasks to run after build:

<Target Name="AfterBuild" DependsOnTargets="RunYarn;CopyBin;GetFiles;DebugProject;Package"></Target>

According to MSBuild docs:

The first target to execute is specified at run time. Targets can have dependencies on other targets. For example, a target for deployment depends on a target for compilation. The MSBuild engine executes dependencies in the order in which they appear in the DependsOnTargets attribute, from left to right.

Therefore, we'd expect RunYarn to execute before Package, but it's not. So, maybe the docs are wrong, or maybe something is getting overwritten and the source of one of these instructions is actually coming from somewhere else.

Does anyone know of a good way to trace what MSBuild thinks it's doing?

@donker donker requested a review from mitchelsellers October 24, 2019 17:43
@donker
Copy link
Contributor Author

donker commented Oct 24, 2019

The new build's artifacts are still missing /Install/Module/Dnn.PersonaBar.UI.09.04.02_Install.zip/Resources.zip/scripts/exports/export-bundle.js (otherwise, it appears to be complete).

Oddly enough I can't replicate this. I just recloned and just ran the build script.

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

Definitely weird. You're just getting a fresh clone and running ./build.ps1, and nothing more?

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

It seems like it could be a timing issue, but it's weird that it seems consistent on DevOps and my machine. Are you using VS 2019 or 2017? I wonder if that might be a contributing factor. I'm using 2019 (and I asssume DevOps is also).

@donker
Copy link
Contributor Author

donker commented Oct 24, 2019

Definitely weird. You're just getting a fresh clone and running ./build.ps1, and nothing more?

Completely fresh clone. Then:

.\Build.ps1 -Target BuildAll

So that should be equivalent, yes. I have both 2017 and 2019 installed on my machine. But build is called from the scripts so cannot see how that is a factor.

@valadas
Copy link
Contributor

valadas commented Oct 24, 2019

Yes and then the next builds are fine. It only happens on the first build. Same happens in CI.

@donker
Copy link
Contributor Author

donker commented Oct 24, 2019

So in answer to @bdukes : I compared my successful build log with the one in DevOps and in my case the zip was created on line 8587 and the export-bundle on 8331. So it seems they are indeed reversed.

But ... but. On a rebuild it's reversed. Hmm.

@bdukes
Copy link
Contributor

bdukes commented Oct 24, 2019

Alright, I think I've tracked this down. Not quite sure what the right answer is.

While the Package task depends on the RunYarn task, RunYarn has a condition Condition="$(YarnWorkingDirectory.Length) > 0 AND '$(Configuration)|$(Platform)' == 'Release|AnyCPU'". The variable YarnWorkingDirectory is only set in Dnn.PersonaBar.Extensions/Module.build. So, there is nothing indicating that yarn build needs to run before the packaging of Dnn.PersonaBar.UI.

The easiest solution is probably to move the Yarn commands into Cake, and make sure they're called before CompileSource. Another (unsatisfying) solution would be to run yarn build for both projects. There's probably a way within MSBuild to get these two pieces to connect, but I don't know what it is.

@donker
Copy link
Contributor Author

donker commented Oct 24, 2019

Together with Daniel we worked at this and added the PersonaBar.Extensions project as dependency for the PersonaBar.UI project.

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to report that it now succeeded in CI, locally and also installed a site with it and it works.
Great job!

Copy link
Contributor

@mitchelsellers mitchelsellers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally and was successful, and CI build result is good! Thanks for all the work

Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time for a rebase again? @bdukes ?

@bdukes
Copy link
Contributor

bdukes commented Oct 25, 2019

Conflict has been addressed, once it's done building let's go ahead and approve this PR with a squash (I'm heading out of town, so don't wait for me to push the squash button)

@bdukes bdukes merged commit 6cec560 into dnnsoftware:release/9.4.x Oct 25, 2019
@david-poindexter
Copy link
Contributor

Thanks @bdukes !

And thanks @donker and @valadas for working on this. It is great!!!

@donker donker deleted the reorg1 branch October 25, 2019 18:59
@valadas valadas added this to the 9.4.2 milestone Nov 4, 2019
donker pushed a commit that referenced this pull request Nov 15, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* Fixes duplication in DotNetNuke.Website.csproj

* Removed reference to missing LeftMenu files
valadas pushed a commit that referenced this pull request Dec 3, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* DNN-32474 - Recursive delete option is added (#3249)

* Move Country Above Region in UserProfile.cs

* spacing issue resolved.

* I keep saving but github is moving it on its own.

* Fixed tab for spaces to resolve spacing issues

* Add PortalSettings overloads back (#3295)

Fixes #3289

* Updates issue templates as per 9.4.2 release

* Move Email Above Username in User.ascx #3305 (#3306)

* Move Email Above Username in User.ascx

* Fixed tab index values in User.ascx

* Fix NuGet warning NU5048 (#3304)

The iconUrl property of the nuspec file is deprecated, it's recommended
to add an icon property, which points to a file within the package,
while also retaining the iconUrl property for older clients.
See https://docs.microsoft.com/en-us/nuget/reference/nuspec#iconurl

* Word-Break added to Journal P (#3307)

* Set core object versions to core version (#3287)

Ensure skin and radeditorprovider get the core version nr and add SPROC to set the version of all core packages/desktopmodules to the core version

Fixes #3239

* Third installment of build reorganization (#3285)

* Improve versioning

* New approach: keep versions in source control and change upon CI build

* Remove previous versioning logic

* Cleaning up tasks

* Fixes

* Ensure UpdateDnnManifests targets just the right manifests

* Harmonize naming of zips and nuget packages

* Testing webpack building to dev website

* Adjust webpack projects in AE to build to dev site

* Fixes

* Delete duplicate stuff

* This is generated by Webpack and shouldn't be tracked

* Include missing project from Lerna

* Fixes for building SitesListView

* Update to packages

* Further cleaning

* Remove RadEditorProvider

* Fix ModuleSettings > Added to Pages grid, pager wasn't working

* Allow only alphanumeric characters to be used when forming url from website name (#3316)

* Ensure UpdateDnnManifests doesn't run for building dev site (#3314)

* Reload page when the code tells you to (#3315)

* Reload page when the code tells you to

* Optimize code

* Google Analytics TrackingID is stored in lowercase #3318 (#3322)

* Update sums.resources

* Fix casing mismatch errors in security analyzer file compare

* Update Dnn.AdminExperience/Extensions/Content/Dnn.PersonaBar.Extensions/Components/Security/Checks/CheckDefaultPage.cs

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Registered IPersonaBarContainer with DI (#3329)

* Document approval process and group

* Fix typo

* Relaxed nuget.cake wildcard (#3331)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants