Dynamo icon indicating copy to clipboard operation
Dynamo copied to clipboard

bring in latest protogeometry, drop asm 227, support min version asm 228.6

Open mjkkirschner opened this issue 3 years ago • 6 comments

Purpose

This PR:

  • brings in the latest protogeometry and libg 228 binaries
  • drops support for asm 227, but keeps empty dummy files around in the libg_227_0_0 folder to avoid breaking patch installers.
  • Makes some changes to the preloader logic:
      1. We no longer will consider older asm versions valid installations - for example if we supply version 228.6 as valid, we will not try to load 228.1 - I found this was a problem locally because we would attempt to load Revit 2023's ASM version instead of falling back to the ASM binaries in the libG folders, making the fallback useless.
      1. When we fallback to look for ASM in the libG folders, we'll look for any major matching folder name - this is not strictly necessary, but it's a bit more flexible and matches the behavior when we do locate an ASM installation - in this case it let's us avoid having a libg_228_0_0 (full of dummy files) and lib_228_6_0 folder.

⚠️ ⚠️ ⚠️

  1. For this PR to pass we'll need to first update the CI scripts that pull ASM onto the master-15 and master-5 build machines.
  2. To get geometry working devs will need to either pull asm 228.6 and use the -gp flag to point to it, throw it in the libG 228 folder, or get a dev installation of a host product that is using the new binaries.

TODO:

  • [x] update master-15 build script
  • [x] update master-5 build script
  • [ ] update API / integration changes docs.
  • [ ] update .md tests
  • [ ] fix failing tests

related PRs: https://git.autodesk.com/Dynamo/DynamoBuildscripts/pull/31 https://git.autodesk.com/Dynamo/DynamoSelfServe/pull/80

Declarations

Check these if you believe they are true

  • [ ] The codebase is in a better state after this PR
  • [ ] Is documented according to the standards
  • [ ] The level of testing this PR includes is appropriate
  • [ ] User facing strings, if any, are extracted into *.resx files
  • [ ] All tests pass using the self-service CI.
  • [ ] Snapshot of UI changes, if any.
  • [ ] Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • [ ] This PR modifies some build requirements and the readme is updated

Release Notes

Drop support for asm 227, PolyCurve enhancements Minimum volume bounding boxes,

Reviewers

mjkkirschner avatar Aug 11 '22 18:08 mjkkirschner

To get geometry working devs will need to either pull asm 228.6 and use the -gp flag to point to it, throw it in the libG 228 folder, or get a dev installation of a host product that is using the new binaries.

@mjkkirschner won't updating the CI scripts ensure that ASM 228.6 is pulled and dumped into the LibG 228 folder or do they rely on a host being installed on the machine?

aparajit-pratap avatar Aug 11 '22 18:08 aparajit-pratap

Drop support for asm 227, bring in polycurve enhancements to Dynamo.

I think this drop will also contain the bounding box enhancements.

aparajit-pratap avatar Aug 11 '22 18:08 aparajit-pratap

I suspect that .md file tests will need to be updated but we can look at those once the tests fail.

aparajit-pratap avatar Aug 11 '22 18:08 aparajit-pratap

@mjkkirschner won't updating the CI scripts ensure that ASM 228.6 is pulled and dumped into the LibG 228 folder or do they rely on a host being installed on the machine?

those scripts only work on CI, they don't rely on a host, but they will not help devs locally.

mjkkirschner avatar Aug 11 '22 18:08 mjkkirschner

@mjkkirschner won't updating the CI scripts ensure that ASM 228.6 is pulled and dumped into the LibG 228 folder or do they rely on a host being installed on the machine?

those scripts only work on CI, they don't rely on a host, but they will not help devs locally.

Ok, I thought you were talking about CI.

aparajit-pratap avatar Aug 11 '22 18:08 aparajit-pratap

There are some test failures here I am looking into.

mjkkirschner avatar Aug 12 '22 12:08 mjkkirschner

@aparajit-pratap @sm6srw @pinzart90 please take another look - I've reverted the min version of ASM to 228.5 - and done basic sanity tests with 228.6 and 228.5 asm, some geometry graphs open and execute which is expected with either set of binaries.

We still have the failure related to volume in the Dynamo.Tests.GeometryDefectTests.TestSweepAsSolid.

but I think all other failures should be resolved - I'll keep an eye on tests.

@aparajit-pratap did you want me to update the test jobs to pull asm 228.5 instead of 228.6?

mjkkirschner avatar Aug 12 '22 17:08 mjkkirschner

@aparajit-pratap @sm6srw @pinzart90 please take another look - I've reverted the min version of ASM to 228.5 - and done basic sanity tests with 228.6 and 228.5 asm, some geometry graphs open and execute which is expected with either set of binaries.

We still have the failure related to volume in the Dynamo.Tests.GeometryDefectTests.TestSweepAsSolid.

but I think all other failures should be resolved - I'll keep an eye on tests.

@aparajit-pratap did you want me to update the test jobs to pull asm 228.5 instead of 228.6?

@mjkkirschner thanks! I'm looking at TestSweepAsSolid right now. Yes, if you could update the test jobs to 228.5 that will be great just so we can be on the safe side. Sorry for the rework.

aparajit-pratap avatar Aug 12 '22 18:08 aparajit-pratap

@aparajit-pratap to confirm, test jobs are updated to 228.5, and all tests are fixed except for: https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/6791/testReport/junit/Dynamo.Tests/GeometryDefectTests/TestSweepAsSolid/

mjkkirschner avatar Aug 12 '22 21:08 mjkkirschner