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

InterceptableLocation.GetDisplayLocation breaks deterministic build #76126

Closed
ThomsonTan opened this issue Nov 27, 2024 · 2 comments · Fixed by #76449
Closed

InterceptableLocation.GetDisplayLocation breaks deterministic build #76126

ThomsonTan opened this issue Nov 27, 2024 · 2 comments · Fixed by #76449

Comments

@ThomsonTan
Copy link

ThomsonTan commented Nov 27, 2024

Version Used: .NET SDK 9.0.100

The current implementation of InterceptableLocation.GetDisplayName() which is used in source generator to output human-readable location for the generated code, but it outputs full path which breaks deterministic build as the the generated source code could be embedded to the PDB file. Should the output location be normalized to restore the deterministic build?

public override string GetDisplayLocation()
{
// e.g. `C:\project\src\Program.cs(12,34)`
return $"{_path}({_lineNumberOneIndexed},{_characterNumberOneIndexed})";
}

/// Gets a human-readable representation of the location, suitable for including in comments in generated code.
/// </summary>
public abstract string GetDisplayLocation();

The generated non-deterministic source code looks like below.

    [GeneratedCode("Microsoft.Extensions.Configuration.Binder.SourceGeneration", "9.0.11.2809")]
    file static class BindingExtensions
    {
        #region IConfiguration extensions.
        /// <summary>Attempts to bind the given object instance to configuration values by matching property names against configuration keys recursively.</summary>
        [InterceptsLocation(1, "aSfEOMF93KOjHfGLzFdenPgFAABPbmVDb2xsZWN0b3JMb2dFeHBvcnRQcm9jZXNzb3JCdWlsZGVyLmNz")] // D:\a\opentelemetry-dotnet-contrib\opentelemetry-dotnet-contrib\src\OpenTelemetry.Exporter.OneCollector\Logs\OneCollectorLogExportProcessorBuilder.cs(47,74)
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 27, 2024
@jjonescz jjonescz changed the title InterceptableLocation.GetDisplayLocation breaks determistic build InterceptableLocation.GetDisplayLocation breaks deterministic build Nov 28, 2024
@jaredpar
Copy link
Member

jaredpar commented Dec 2, 2024

but it outputs full path which breaks deterministic build

A deterministic build is when given the same inputs a build produces the same outputs. The path of source files are part of the input to a build and it's reasonable, and expected, that changing them will result in different build outputs. In the case the user wants to normalize paths the -pathmap feature is available.

That being said @RikkiGibson, in looking through this code I don't see that PathMap is being used here. Why is that? Seems odd.

@RikkiGibson
Copy link
Contributor

It looks like applying pathmap was not specifically discussed during design (#72133) or API review feedback (#72133 (comment)). Nor in review of the implementation (#72814).

I don't recall any reason we would want to avoid pathmap, and it seems like using it is the norm when compiler is outputting a file path.

It should be straightforward to make a bugfix level change which maps the path, same as if the path were appearing in a PDB.

@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Dec 2, 2024
@jaredpar jaredpar added this to the 17.13 milestone Dec 2, 2024
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