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 temp file filtering in FileTracker #8352

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

Forgind
Copy link
Member

@Forgind Forgind commented Jan 25, 2023

(Copied from #8351)

Fixes AB#1678521

Context
#8049 broke the temp filtering logic by using the MSBuild-specific temp path instead of the true base temp path. This manifests as an overbuild of some C++ projects.

Changes Made
Reverted the change. Enabled relevant unit tests.

Testing
Existing unit tests and a C++ end-to-end repro.

Notes
The rest of FileTracker tests cannot be enabled without significant work (related to #649).

@JanKrivanek
Copy link
Member

Btw. would a cherrypick be possible here? The branches doesn't seem to be much diverged: https://github.com/dotnet/msbuild/compare/main...vs17.0?expand=1

If we'd do a similar thing more often then once in a blue moon, than we can introduce backport GHA:
https://github.com/dotnet/arcade/blob/main/.github/workflows/backport-base.yml

@Forgind
Copy link
Member Author

Forgind commented Jan 30, 2023

Btw. would a cherrypick be possible here? The branches doesn't seem to be much diverged: https://github.com/dotnet/msbuild/compare/main...vs17.0?expand=1

If we'd do a similar thing more often then once in a blue moon, than we can introduce backport GHA: https://github.com/dotnet/arcade/blob/main/.github/workflows/backport-base.yml

I initially tried to cherry-pick, but there was apparently a conflict on the tests, so it ended up as a new commit. The 17.5 one went in more cleanly.

@JanKrivanek
Copy link
Member

JanKrivanek commented Jan 30, 2023

Btw. I added a proposal for minimalistic backport action - for non-diverging branches we can then port simply via comment on original PR: #8368

@Forgind Forgind merged commit 2fdde65 into dotnet:vs17.0 Feb 7, 2023
@Forgind Forgind deleted the dial-back-temp-change branch February 7, 2023 17:06
JanKrivanek added a commit that referenced this pull request Apr 26, 2023
commit 81da5a2
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Apr 26 10:17:54 2023 -0700

    [vs17.2] Temporary turn off static graph restore. (#8697)

    Turn off static graph restore.

    (Backport of #8498 to vs17.2)

commit 872ddd9
Merge: 94fcedf 6faa1d5
Author: Forgind <12969783+Forgind@users.noreply.github.com>
Date:   Mon Feb 13 07:30:47 2023 -0800

    Merge pull request #8419 from Forgind/fix-temp-file-filtering-17.2

    Fixes AB#1678521

    Context
    #8049 broke the temp filtering logic by using the MSBuild-specific temp path instead of the true base temp path. This manifests as an overbuild of some C++ projects.

    Changes Made
    Reverted the change. Enabled relevant unit tests.

    Testing
    Existing unit tests and a C++ end-to-end repro.

    Notes
    The rest of FileTracker tests cannot be enabled without significant work (related to #649).

commit 6faa1d5
Merge: 94fcedf 2fdde65
Author: Forgind <Forgind@users.noreply.github.com>
Date:   Tue Feb 7 14:56:15 2023 -0800

    Merge branch 'vs17.0' of https://github.com/dotnet/msbuild into vs17.2

commit 2fdde65
Author: Forgind <12969783+Forgind@users.noreply.github.com>
Date:   Tue Feb 7 09:06:10 2023 -0800

    Fix temp file filtering in FileTracker (#8352)

    (Copied from #8351)

    Fixes AB#1678521

    Context
    #8049 broke the temp filtering logic by using the MSBuild-specific temp path instead of the true base temp path. This manifests as an overbuild of some C++ projects.

    Changes Made
    Reverted the change. Enabled relevant unit tests.

    Testing
    Existing unit tests and a C++ end-to-end repro.

    Notes
    The rest of FileTracker tests cannot be enabled without significant work (related to #649).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants