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

Clean repository build results in: AsnXml regenerated files, be sure to check them in #101651

Closed
bbartels opened this issue Apr 27, 2024 · 8 comments · Fixed by #101657
Closed
Assignees
Milestone

Comments

@bbartels
Copy link
Contributor

bbartels commented Apr 27, 2024

Having followed the instructions here, attempting to build the runtime + libraries I ran into the following errors after running build.cmd clr+libs -rc Release. Upon a second attempt to compile, the build is successful. These two files evidently have some purpose as deleting them will regenerate them again. What is that purpose of these files, how am I suppose to deal with them? The error makes it sound like I am supposed to "check them in"(to git?). I presume that is not the case.

D:\dotnet\runtime\src\libraries\Common\src\System\Security\Cryptography\Asn1\AsnXml.targets(42,5): error : AsnXml regenerated files, be sure to check them in: D:\dotnet\runtime\src\libraries\Common\src\System\Security\Cryptogr
aphy\Asn1\AlgorithmIdentifierAsn.xml.cs [D:\dotnet\runtime\src\libraries\System.Security.Cryptography\src\System.Security.Cryptography.csproj::TargetFramework=net9.0-windows]
D:\dotnet\runtime\src\libraries\Common\src\System\Security\Cryptography\Asn1\AsnXml.targets(42,5): error : AsnXml regenerated files, be sure to check them in: D:\dotnet\runtime\src\libraries\Common\src\System\Security\Cryptogr
aphy\Asn1\SubjectPublicKeyInfoAsn.xml.cs [D:\dotnet\runtime\src\libraries\System.Security.Cryptography\src\System.Security.Cryptography.csproj::TargetFramework=net9.0-windows]
    0 Warning(s)
    2 Error(s)

My suggestion would be to:

  1. Either generate the files silently without failing the build or improve the message by stating that one would have to re-run the build
  2. Have a default .gitignore for these files for them not to pollute the git context or some guidance around how one is suppose to deal with the files

When following the instructions

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 27, 2024
@vcsjones
Copy link
Member

This shouldn't happen in a clean checkout, and I cannot reproduce it (and neither does CI) so it seems to be something that is specific to your local environment.

Generally you should only get this message if you have modified any of the *Asn.xml files. The intention of the error is to say "You modified the XML, you better check in the C# that it generated, too"

These two files evidently have some purpose as deleting them will regenerate them again.

Well, yes. They are types that are used as part of ASN.1 serialization.

Presumably when you build, the AlgorithmIdentifierAsn.xml.cs and SubjectPublicKeyInfoAsn.xml.cs files have changed. If you look at the git differences (git diff) how did the compiler change those files?

@vcsjones vcsjones added the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 28, 2024
@bbartels
Copy link
Contributor Author

While it is an older fresh pull from github, the HEAD is targeting a commit from today(55d2ada) and I ran git clean -xdf to clean any build artefacts. The following is the output of git status & git diff.

PS D:\dotnet\runtime> git rev-parse HEAD
55d2adab3aa7e3fb764a44bf5661cffd21936596
PS D:\dotnet\runtime> git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   src/libraries/Common/src/System/Security/Cryptography/Asn1/AlgorithmIdentifierAsn.xml.cs
        modified:   src/libraries/Common/src/System/Security/Cryptography/Asn1/SubjectPublicKeyInfoAsn.xml.cs

no changes added to commit (use "git add" and/or "git commit -a")
PS D:\dotnet\runtime> git diff
warning: in the working copy of 'src/libraries/Common/src/System/Security/Cryptography/Asn1/AlgorithmIdentifierAsn.xml.cs', CRLF will be replaced by LF the next time Git touches it
warning: in the working copy of 'src/libraries/Common/src/System/Security/Cryptography/Asn1/SubjectPublicKeyInfoAsn.xml.cs', CRLF will be replaced by LF the next time Git touches it

I frankly should have checked what exactly the diff output was. Having ran the build again with: git config --global core.autocrlf false it built successfully first try🤦 Still not entirely sure why any line endings would have changed though, could a change in .gitattributes stop something like this from happening?

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 28, 2024
@vcsjones
Copy link
Member

could a change in .gitattributes stop something like this from happening?

Got it. Yeah our generated code is sensitive to line ending differences, and it sounds like you had git to default to lf line endings instead of "default" line endings.

I think this scenario should work though. I'll take a look at making changes to the .gitattributes so that git knows it really no-kidding needs to use CRLF on Windows.

@vcsjones vcsjones self-assigned this Apr 28, 2024
@vcsjones vcsjones added this to the 9.0.0 milestone Apr 28, 2024
@vcsjones vcsjones removed the untriaged New issue has not been triaged by the area owner label Apr 28, 2024
@danmoseley
Copy link
Member

Just curious, if the build generates then why don't they go to temp? Why check them in? Is it due to some circular dependency?

@filipnavara
Copy link
Member

Why check them in?

The original reason was that the XML transformation task was missing in portable MSBuild so you had to do the generation with desktop MSBuild on Windows. Since the files are used on other platforms they had to be checked in. I don’t think this is a blocker anymore.

@vcsjones
Copy link
Member

From #45210

One of the things that has been great about the current model is that the source files show up on source.dot.net; that's something I'd like to keep true under any change.

So changing that was not something I had considered without input from @bartonjs. Since the current stance is to check them in, the most sensible thing to do is to ignore the line endings when diffing.

@bbartels
Copy link
Contributor Author

Thanks @vcsjones! It's always a bit difficult coming back to contribute whenever I find time and running into small things like this. So happy to see this resolved going forward :)

@vcsjones
Copy link
Member

It's always a bit difficult coming back to contribute whenever I find time and running into small things like this

Definitely. Papercuts like this add up. Thanks for writing an issue to get it fixed.

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.

4 participants