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

PathMap should prefer more specific prefix mapping #46061

Closed
tmat opened this issue Jul 16, 2020 · 4 comments · Fixed by #49670
Closed

PathMap should prefer more specific prefix mapping #46061

tmat opened this issue Jul 16, 2020 · 4 comments · Fixed by #49670
Milestone

Comments

@tmat
Copy link
Member

tmat commented Jul 16, 2020

Currently the mappings are applied in the order they are specified, regardless of whether one prefix is more specific than other.

Thus a mapping specified earlier in the list overrides more specific mapping specified later in the list.

csc /target:library /debug:portable a.cs a\b.cs a\b\c.cs /pathmap:C:\temp\=/_1/,C:\temp\a\=/_2/,C:\temp\a\b\=/_3/

This behavior is not desirable since one might want to specify a SourceRoot that covers a specified directory with the goal to make all paths from such directory deterministic, but other SourceRoots might be available for subdirectories that add Source Link information. By ignoring the subdirectories and normalizing the the containing directory root the Source Link information is lost.

For example, NuGet targets set a SourceRoot for the entire package cache root to make sure source packages do not break deterministic build even when these packages don't define their own SourceRoot. Source packages built with with Source Link enabled do however add their SourceRoot entry (with Source Link metadata). This entry is however overridden in path map due to the above behavior.

Version Used:
3.8.0-1.20360.1 (ec2c0bc)

Steps to Reproduce:

  1. Unzip repro.zip.
  2. Run repro.cmd
  3. Run mdv a.pdb

Expected Behavior:

Document (index: 0x30, size: 24):
============================================================================================================================================================
   Name                   Language  HashAlgorithm  Hash
============================================================================================================================================================
1: '/_1/a.cs' (#84d)      C# (#4)   SHA-256 (#3)   86-6B-7E-F8-37-4C-22-F9-E9-3B-DA-2F-DC-9B-6C-2C-DB-AB-BA-55-F4-AA-77-B9-74-50-52-22-06-31-97-1A (#854)
2: '/_2/b.cs' (#87c)    C# (#4)   SHA-256 (#3)   B8-9B-74-2E-6D-FA-D2-A6-7F-80-7E-29-BA-07-93-0C-D3-5F-97-16-B6-6A-1B-26-21-C8-45-69-D0-96-4B-6C (#885)
3: '/_3/c.cs' (#8ad)  C# (#4)   SHA-256 (#3)   CE-8F-5A-CE-5C-4F-85-A8-81-5E-A4-B9-E0-B2-47-24-BD-FF-C0-4C-68-40-20-8F-A6-47-39-A5-67-2C-9F-3D (#8b8)

Actual Behavior:

Document (index: 0x30, size: 24):
============================================================================================================================================================
   Name                   Language  HashAlgorithm  Hash
============================================================================================================================================================
1: '/_1/a.cs' (#84d)      C# (#4)   SHA-256 (#3)   86-6B-7E-F8-37-4C-22-F9-E9-3B-DA-2F-DC-9B-6C-2C-DB-AB-BA-55-F4-AA-77-B9-74-50-52-22-06-31-97-1A (#854)
2: '/_1/a/b.cs' (#87c)    C# (#4)   SHA-256 (#3)   B8-9B-74-2E-6D-FA-D2-A6-7F-80-7E-29-BA-07-93-0C-D3-5F-97-16-B6-6A-1B-26-21-C8-45-69-D0-96-4B-6C (#885)
3: '/_1/a/b/c.cs' (#8ad)  C# (#4)   SHA-256 (#3)   CE-8F-5A-CE-5C-4F-85-A8-81-5E-A4-B9-E0-B2-47-24-BD-FF-C0-4C-68-40-20-8F-A6-47-39-A5-67-2C-9F-3D (#8b8)
@nkolev92
Copy link

Is this still scheduled for 16.8 @tmat

Trying to understand if I should pull in NuGet/Home#9810, into the next sprint.

@tmat
Copy link
Member Author

tmat commented Aug 17, 2020

Not sure if we can make it. It might come late in 16.8.

@nkolev92
Copy link

Ok, thanks, I'll revisit next sprint.

@tmat
Copy link
Member Author

tmat commented Dec 10, 2020

@nkolev92 Fix is now merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants