Order assembly loading deterministically - #3
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This does not fix CI_Toolkit#161. It makes the collision deterministic, not correct. Read the title as "the answer no longer depends on the filesystem", not as "the right assembly now answers".
ProbeDeclaringTypetakes its verdict from the first loaded assembly that yields the declaring type.Directory.GetFilesdocuments no ordering, so on any closure where two assemblies define the same type name the classification depended on the filesystem. That is #161's mechanism, and it is not a single case: 42 type names across the fleet are defined by more than one assembly, 9 of them across repository boundaries.After this change
Revit_Core_Engine_2022wins that contest consistently, because C sorts before M. That is the right claimant when the subject isRevit_Toolkitand the wrong one when the subject is the other repo, and it is now wrong reproducibly rather than intermittently. The 811 dataset entries that aRevit_ModelQA_Toolrun would attribute to the wrong repo are untouched by this. #161 stays open and still needs either declaring-assembly attribution or its option 3, moving the colliding type into its own namespace at source.What this does buy is that the remaining problem no longer moves under measurement, which is a precondition for the differential work rather than a fix in itself.
Measured on
windows-2025-vs2026across four cold-rebuild runs on separate runners, NTFS returned exactlyStringComparer.OrdinalIgnoreCaseorder every time, 132 and 111 entries. So this is a no-op there and the change is defensive. It converts an observation about one filesystem into a property of the code.The comparer is not interchangeable, which is why a test names it. On the real 132-assembly closure,
Ordinal,OrdinalIgnoreCaseand a lowercase-based sort each produce a different order, and onlyOrdinalIgnoreCasereproduces what NTFS returned. It uppercases before comparing, so_(0x5F) lands after letters andRevitAPIUIprecedesRevit_Adapter; a lowercase sort reverses that pair and would change which assembly answers for a contested type.Eight tests, four mutations, each caught by the intended one: no sort, lowercase sort, plain
Ordinal, and sorting the whole path instead of the file name.