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

Fix ignored paths used by binary tool #40777

Merged
merged 8 commits into from May 10, 2024

Conversation

ellahathaway
Copy link
Member

Closes dotnet/source-build#4391

The binary tool was not detecting binaries in repos where a .git repo was not present, because all files were being ignored upon initialization of said git repo.

Furthermore, some repos have been including binaries in their .gitignore files. This results in those binaries going undetected by the tool. See https://github.com/dotnet/dotnet/blob/8f3d7c26c231d54ad230f427a1005278c815dde6/src/fsharp/.gitignore#L43 as an example.

This PR changes the ignored paths from relying on the .gitignore files to simply hardcoding paths to ignore.

@ellahathaway ellahathaway requested review from a team as code owners May 8, 2024 18:11
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels May 8, 2024
@ellahathaway ellahathaway requested a review from mthalman May 8, 2024 21:57
Copy link
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we ignoring certain binaries anyway? If we're not going to honor the .gitignore files of the repos, then what service are we providing to the developer anyway by keeping some binaries in the repo but not others?

@ellahathaway
Copy link
Member Author

Why are we ignoring certain binaries anyway? If we're not going to honor the .gitignore files of the repos, then what service are we providing to the developer anyway by keeping some binaries in the repo but not others?

My only thought is that we can't rely on the .gitignore files, but we should still ignore certain paths like the git files, artifacts, and downloaded PSB packages. If someone wants to run just the validation for example, it would be unfortunate if the tool removed their build artifacts.

@MichaelSimons
Copy link
Member

MichaelSimons commented May 9, 2024

Per an offline discussion w/@ella, we concluded the following:

  1. The use case of binary detection/stripping is from a clean environment. If you run the tool in a built environment, it is alright to detect all the built assets.
  2. You can't trust/depend on .gitignore files because they can contain checked in files and we can't depend on the tool running in context of git
  3. It is nice to have one place with the exclusions. Having certain paths hard coded in the tool in addition to the allowed-binaries txt file makes understanding the behavior more difficult.
  4. We need to exclude certain well known paths in order for the tool to work correctly from the prep script - .git, .dotnet, .packages, prereqs/packages

@ellahathaway ellahathaway enabled auto-merge (squash) May 10, 2024 18:03
@ellahathaway ellahathaway merged commit d5cb00f into dotnet:main May 10, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Binaries are not being removed from VMR when building from archive
4 participants