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

feat: Create a Build task for DsCOM #102

Merged

Conversation

carstencodes
Copy link
Member

@carstencodes carstencodes commented Dec 1, 2022

Preface

DsCom is cool, but it lacks support for build within msbuild.

Details

This pull requests adds a nuget package that cares about exporting the Assembly to a type library automatically.
Do to a limitation in msbuild, it is not properly possible to load dependent assemblies in build task.
As suggested by Nate McMaster the task is now bundled in two ways (cf. src\dscom.build\DsComPackaging.targets ):

  • The build task comes as a native MsBuild Task. It bundles dSPACE.Runtime.InteropServices and loads the file automatically.
  • The targets for .NET Standard 2.0 and x86 call the DsCom Exe. The Exe file is bundled within the package using a SingleExecutable (i.e. with IL Merged InteropServices-Assembly) in both architectures - 32 and 64 bit.

Features

  • Call TypeLibrary Converter within an MsBuild task
  • Add Verbose and diagnostic logging
  • Check, if desired TLB exists after the build
    • Provide a warning, if the type library file is missing after the build
    • Add a code to the warning to provide the user setting WarningsAsError (DSCOM001)
  • Allow override for the build task to use Tools
  • Automatically determine name and path of the targeted type library from the targeted assembly
  • Add Included Reference Files to Assembly path

Quality Assurance

  • Unit tests are written. They are now marked explicitly as a single Pull Request Action.
  • The x86 example serves as Acceptance test
  • The OutProc example serves as Acceptance test
  • The acceptance tests are part of a pull request

Continuous Integraion

  • The Code Style check has been moved to a new location
  • The build task package is packed during the build
  • The build task package is pushed to nuget.org during the release

Documentation

  • A ReadMe.md is provided by @carstencodes
  • The description for the NuGet Package is set to the readme.md
  • Add architecture decision records
  • Add a documentation for the implementation

Known Limitations

  • Resulting package is not marked as .NET Standard 2.0 compliant. Clients must suppress NU1701 for this package, if the Assembly targets .NET Standard 2.0 (cf. OutProc Example). A bug must be filed. @carstencodes Severity: Major
  • In some cases the build does not provide a type library. (cf. Outproc Example). A bug must be filed. @carstencodes Severity Major

Contained fixes

  • RegistryHelper of OutProc Example contained a cast from nullable to non-nullable (Compiler Warning)

Review

@carstencodes carstencodes marked this pull request as ready for review December 1, 2022 14:11
@carstencodes carstencodes force-pushed the contrib/carstencodes/dscom-build-task branch from a8b2730 to 45b5f00 Compare December 5, 2022 19:57
docs/adr/ReadMe.md Outdated Show resolved Hide resolved
Copy link
Member

@marklechtermann marklechtermann left a comment

Choose a reason for hiding this comment

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

Hello Carsten,

as discussed the following change requests:

  • docs should be removed
  • docs/dscom.build/ReadMe.md should be moved to src/dscom.build/docs/README.md
  • the content of src/dscom.build/ReadMe.md should be moved to README.md

Thx, Great work!

src/dscom.build/dscom.build.csproj Outdated Show resolved Hide resolved
docs/ReadMe.md Outdated Show resolved Hide resolved
docs/dscom.build/ReadMe.md Outdated Show resolved Hide resolved
docs/dscom.build/ReadMe.md Outdated Show resolved Hide resolved
@carstencodes
Copy link
Member Author

Hello Carsten,

as discussed the following change requests:

  • docs should be removed
  • docs/dscom.build/ReadMe.md should be moved to src/dscom.build/docs/README.md
  • the content of src/dscom.build/ReadMe.md should be moved to README.md

Thx, Great work!

I appreciate your feedback. I have submitted e36ea67 and 04afc4b containing the requested changes.

carstencodes added a commit to carstencodes/dscom that referenced this pull request Dec 14, 2022
Entries that are part of Directory.Build.props
are removed from csproj.
This was requested by @SOsterbrink in dspace-group#102.
@carstencodes carstencodes requested review from SOsterbrink and removed request for marklechtermann December 14, 2022 19:17
carstencodes and others added 14 commits December 14, 2022 20:18
MsBuild fails to host dSPACE.Runtime.InteropServices for TLB Export
Use the dependency property instead of AfterTargets declaration.
The dscom build documentation is in a reviewable format.
Should do as a first draft.
As requested by @marklechtermann the
user documentation has been moved
to the common section.
As requested by @marklechtermann and @SOsterbrink the documentation
folder with the unmaintained
documentation was removed while the
documentation of the build task
was moved to the src folder
Entries that are part of Directory.Build.props
are removed from csproj.
This was requested by @SOsterbrink in dspace-group#102.
@carstencodes carstencodes force-pushed the contrib/carstencodes/dscom-build-task branch from fdb17a3 to a77b8c3 Compare December 14, 2022 19:18
@carstencodes
Copy link
Member Author

Mentioned commits have changed. As requested by @marklechtermann I have rebased the branch onto the latest upstream main branch. Content still stays the same.

The recent three commits should contain the requested changes.

@carstencodes carstencodes requested review from marklechtermann and removed request for SOsterbrink December 14, 2022 19:20
Copy link
Member

@SOsterbrink SOsterbrink left a comment

Choose a reason for hiding this comment

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

Looks good. I added a few questions to better understand your work.
Thanks for your work and your attention to detail.

README.md Show resolved Hide resolved
src/dscom.build/doc/ReadMe.md Outdated Show resolved Hide resolved
src/dscom.build/doc/ReadMe.md Outdated Show resolved Hide resolved
src/dscom.build/doc/ReadMe.md Outdated Show resolved Hide resolved
src/dscom.build/doc/ReadMe.md Outdated Show resolved Hide resolved
The links have been updated to match the new location.
The document now contains one sentence per line as requested by @SOsterbrink
@SOsterbrink SOsterbrink merged commit 087e6fc into dspace-group:main Dec 15, 2022
@carstencodes carstencodes deleted the contrib/carstencodes/dscom-build-task branch December 15, 2022 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants