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

[Xamarin.Android.Build.Tasks] build.props can cause ConvertResourceCases to skip #1943

Conversation

jonathanpeppers
Copy link
Member

Fixes: #1933

There have been various reports of a scenario such as:

  • Open a project in VS 2017, that uses uppercase letters when
    referencing a resource, such as @drawable/IMALLCAPS
  • A Design-Time Build occurs
  • Some future build later, a regular build fails with an APT0000
    error
    Resources\layout\test.axml(3,0): error APT0000: No resource found that matches the given name (at 'src' with value '@drawable/IMALLCAPS')

Once you are in this state, you are basically hosed until you
Rebuild or delete obj...

After a lot of digging, I was able to reproduce the issue by
invalidating build.props. This happens in a lot of common
situations:

  • $(DesignTimeBuild) changes, such as a DTB or a regular build
  • $(AdbTarget) changes, such as when you change the device or
    emulator you are deploying to

I created a BuildPropsBreaksConvertResourcesCases test that
reproduces this issue by modifying the timestamp on build.props.

If build.props was invalidated, this was happening:

Target Name=_UpdateAndroidResgen Project=UnnamedProject.csproj
...
    Target Name=_GenerateAndroidResourceDir Project=UnnamedProject.csproj
        Building target "_GenerateAndroidResourceDir" completely.
        Input file "obj\Debug\build.props" is newer than output file "obj\Debug\res.flag".
        ...
            CopyIfChanged Task
              ...
              ModifiedFiles:
                C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\main.xml
                C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\test.xml
                C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\values\strings.xml

The CopyIfChanged task is overwriting my test.xml layout file with
the original file contents--which has IMALLCAPS in uppercase.

This would normally be OK, except ConvertResourcesCases skipped
processing this file!

ConvertResourcesCases
    Parameters
        ResourceDirectories = obj\Debug\res\
        AcwMapFile = obj\Debug\acw-map.txt
        AndroidConversionFlagFile = obj\Debug\R.cs.flag
    ConvertResourcesCases Task
      ResourceDirectories: obj\Debug\res\
      AcwMapFile: obj\Debug\acw-map.txt
      AndroidConversionFlagFile: obj\Debug\R.cs.flag
      AndroidConversionFlagFile modified: 7/9/2018 7:57:56 PM
      Skipping: obj\Debug\res\layout\main.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
      Skipping: obj\Debug\res\layout\test.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
      Skipping: obj\Debug\res\values\strings.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM

Since obj\Debug\R.cs.flag is newer than these files, IMALLCAPS is
not converted to lower case!

It appears that the CopyIfChanged MSBuild task was not setting the
timestamps of files it copies...

After fixing this, CopyIfChanged does not ovewrite test.xml in the
first place:

CopyIfChanged Task
  ...
  Skipping Resources\drawable-hdpi\Icon.png its up to date
  Skipping Resources\drawable-mdpi\Icon.png its up to date
  Skipping Resources\drawable-xhdpi\Icon.png its up to date
  Skipping Resources\drawable-xxhdpi\Icon.png its up to date
  Skipping Resources\drawable-xxxhdpi\Icon.png its up to date
  Skipping Resources\drawable\IMALLCAPS.png its up to date
  Skipping Resources\layout\Main.axml its up to date
  Skipping Resources\layout\test.axml its up to date
  Skipping Resources\values\Strings.xml its up to date
ModifiedFiles:

Note that ModifiedFiles is blank, and my unit test now passes.

…ses to skip

Fixes: dotnet#1933

There have been various reports of a scenario such as:
- Open a project in VS 2017, that uses uppercase letters when
  referencing a resource, such as `@drawable/IMALLCAPS`
- A Design-Time Build occurs
- Some future build later, a *regular* build fails with an `APT0000`
  error

    Resources\layout\test.axml(3,0): error APT0000: No resource found that matches the given name (at 'src' with value '@drawable/IMALLCAPS')

Once you are in this state, you are basically hosed until you
`Rebuild` or delete `obj`...

After a lot of digging, I was able to reproduce the issue by
invalidating `build.props`. This happens in a lot of common
situations:
- `$(DesignTimeBuild)` changes, such as a DTB or a regular build
- `$(AdbTarget)` changes, such as when you change the device or
  emulator you are deploying to

I created a `BuildPropsBreaksConvertResourcesCases` test that
reproduces this issue by modifying the timestamp on `build.props`.

If `build.props` was invalidated, this was happening:

    Target Name=_UpdateAndroidResgen Project=UnnamedProject.csproj
    ...
        Target Name=_GenerateAndroidResourceDir Project=UnnamedProject.csproj
            Building target "_GenerateAndroidResourceDir" completely.
            Input file "obj\Debug\build.props" is newer than output file "obj\Debug\res.flag".
            ...
                CopyIfChanged Task
                  ...
                  ModifiedFiles:
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\main.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\test.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\values\strings.xml

The `CopyIfChanged` task is overwriting my `test.xml` layout file with
the original file contents--which has `IMALLCAPS` in uppercase.

This would normally be OK, except `ConvertResourcesCases` skipped
processing this file!

    ConvertResourcesCases
        Parameters
            ResourceDirectories = obj\Debug\res\
            AcwMapFile = obj\Debug\acw-map.txt
            AndroidConversionFlagFile = obj\Debug\R.cs.flag
        ConvertResourcesCases Task
          ResourceDirectories: obj\Debug\res\
          AcwMapFile: obj\Debug\acw-map.txt
          AndroidConversionFlagFile: obj\Debug\R.cs.flag
          AndroidConversionFlagFile modified: 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\main.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\test.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\values\strings.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM

Since `obj\Debug\R.cs.flag` is newer than these files, `IMALLCAPS` is
not converted to lower case!

It appears that the `CopyIfChanged` MSBuild task was not setting the
timestamps of files it copies...

After fixing this, `CopyIfChanged` does not ovewrite `test.xml` in the
first place:

    CopyIfChanged Task
      ...
      Skipping Resources\drawable-hdpi\Icon.png its up to date
      Skipping Resources\drawable-mdpi\Icon.png its up to date
      Skipping Resources\drawable-xhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxxhdpi\Icon.png its up to date
      Skipping Resources\drawable\IMALLCAPS.png its up to date
      Skipping Resources\layout\Main.axml its up to date
      Skipping Resources\layout\test.axml its up to date
      Skipping Resources\values\Strings.xml its up to date
    ModifiedFiles:

Note that `ModifiedFiles` is blank, and my unit test now passes.
@dellis1972
Copy link
Contributor

looks ok. Lets see what the unit tests spit out. If all green then we should merge it

@jonathanpeppers jonathanpeppers merged commit 7f6750f into dotnet:master Jul 9, 2018
@jonathanpeppers jonathanpeppers deleted the buildprops-convertresourcescases branch July 9, 2018 23:29
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jul 12, 2018
Context: dotnet#1933
Context: dotnet#1943

One of the issues I noticed while debugging the issue with dotnet#1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `designtimebuild.props` that works
independantly of `build.props`. This prevents some slower targets from
running when they shouldn't, such as `_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jul 13, 2018
Fixes: dotnet#1958
Context: dotnet#1933
Context: dotnet#1943

One of the issues I noticed while debugging the issue with dotnet#1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
- `CleanDependsOn` also needs to specify the
  `_CleanDesignTimeIntermediateDir` target, I updated the
  `BuildApplicationAndClean` test to verify this
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jul 17, 2018
Fixes: dotnet#1958
Context: dotnet#1933
Context: dotnet#1943

One of the issues I noticed while debugging the issue with dotnet#1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
dotnet@34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jul 17, 2018
Fixes: dotnet#1958
Context: dotnet#1933
Context: dotnet#1943

One of the issues I noticed while debugging the issue with dotnet#1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
dotnet@34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
dellis1972 pushed a commit that referenced this pull request Jul 19, 2018
Fixes: #1958
Context: #1933
Context: #1943

One of the issues I noticed while debugging the issue with #1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
jonpryor pushed a commit to jonpryor/xamarin-android that referenced this pull request Jul 21, 2018
…ses to skip (dotnet#1943)

Fixes: dotnet#1933

There have been various reports of a scenario such as:
- Open a project in VS 2017, that uses uppercase letters when
  referencing a resource, such as `@drawable/IMALLCAPS`
- A Design-Time Build occurs
- Some future build later, a *regular* build fails with an `APT0000`
  error

    Resources\layout\test.axml(3,0): error APT0000: No resource found that matches the given name (at 'src' with value '@drawable/IMALLCAPS')

Once you are in this state, you are basically hosed until you
`Rebuild` or delete `obj`...

After a lot of digging, I was able to reproduce the issue by
invalidating `build.props`. This happens in a lot of common
situations:
- `$(DesignTimeBuild)` changes, such as a DTB or a regular build
- `$(AdbTarget)` changes, such as when you change the device or
  emulator you are deploying to

I created a `BuildPropsBreaksConvertResourcesCases` test that
reproduces this issue by modifying the timestamp on `build.props`.

If `build.props` was invalidated, this was happening:

    Target Name=_UpdateAndroidResgen Project=UnnamedProject.csproj
    ...
        Target Name=_GenerateAndroidResourceDir Project=UnnamedProject.csproj
            Building target "_GenerateAndroidResourceDir" completely.
            Input file "obj\Debug\build.props" is newer than output file "obj\Debug\res.flag".
            ...
                CopyIfChanged Task
                  ...
                  ModifiedFiles:
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\main.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\test.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\values\strings.xml

The `CopyIfChanged` task is overwriting my `test.xml` layout file with
the original file contents--which has `IMALLCAPS` in uppercase.

This would normally be OK, except `ConvertResourcesCases` skipped
processing this file!

    ConvertResourcesCases
        Parameters
            ResourceDirectories = obj\Debug\res\
            AcwMapFile = obj\Debug\acw-map.txt
            AndroidConversionFlagFile = obj\Debug\R.cs.flag
        ConvertResourcesCases Task
          ResourceDirectories: obj\Debug\res\
          AcwMapFile: obj\Debug\acw-map.txt
          AndroidConversionFlagFile: obj\Debug\R.cs.flag
          AndroidConversionFlagFile modified: 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\main.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\test.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\values\strings.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM

Since `obj\Debug\R.cs.flag` is newer than these files, `IMALLCAPS` is
not converted to lower case!

It appears that the `CopyIfChanged` MSBuild task was not setting the
timestamps of files it copies...

After fixing this, `CopyIfChanged` does not ovewrite `test.xml` in the
first place:

    CopyIfChanged Task
      ...
      Skipping Resources\drawable-hdpi\Icon.png its up to date
      Skipping Resources\drawable-mdpi\Icon.png its up to date
      Skipping Resources\drawable-xhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxxhdpi\Icon.png its up to date
      Skipping Resources\drawable\IMALLCAPS.png its up to date
      Skipping Resources\layout\Main.axml its up to date
      Skipping Resources\layout\test.axml its up to date
      Skipping Resources\values\Strings.xml its up to date
    ModifiedFiles:

Note that `ModifiedFiles` is blank, and my unit test now passes.
jonpryor added a commit that referenced this pull request Jul 22, 2018
…ses to skip (#1943) (#1987)

Fixes: #1933

There have been various reports of a scenario such as:
- Open a project in VS 2017, that uses uppercase letters when
  referencing a resource, such as `@drawable/IMALLCAPS`
- A Design-Time Build occurs
- Some future build later, a *regular* build fails with an `APT0000`
  error

    Resources\layout\test.axml(3,0): error APT0000: No resource found that matches the given name (at 'src' with value '@drawable/IMALLCAPS')

Once you are in this state, you are basically hosed until you
`Rebuild` or delete `obj`...

After a lot of digging, I was able to reproduce the issue by
invalidating `build.props`. This happens in a lot of common
situations:
- `$(DesignTimeBuild)` changes, such as a DTB or a regular build
- `$(AdbTarget)` changes, such as when you change the device or
  emulator you are deploying to

I created a `BuildPropsBreaksConvertResourcesCases` test that
reproduces this issue by modifying the timestamp on `build.props`.

If `build.props` was invalidated, this was happening:

    Target Name=_UpdateAndroidResgen Project=UnnamedProject.csproj
    ...
        Target Name=_GenerateAndroidResourceDir Project=UnnamedProject.csproj
            Building target "_GenerateAndroidResourceDir" completely.
            Input file "obj\Debug\build.props" is newer than output file "obj\Debug\res.flag".
            ...
                CopyIfChanged Task
                  ...
                  ModifiedFiles:
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\main.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\layout\test.xml
                    C:\Users\myuser\Desktop\Git\xamarin-android\bin\TestDebug\temp\BuildPropsBreaksConvertResourcesCases\obj\Debug\res\values\strings.xml

The `CopyIfChanged` task is overwriting my `test.xml` layout file with
the original file contents--which has `IMALLCAPS` in uppercase.

This would normally be OK, except `ConvertResourcesCases` skipped
processing this file!

    ConvertResourcesCases
        Parameters
            ResourceDirectories = obj\Debug\res\
            AcwMapFile = obj\Debug\acw-map.txt
            AndroidConversionFlagFile = obj\Debug\R.cs.flag
        ConvertResourcesCases Task
          ResourceDirectories: obj\Debug\res\
          AcwMapFile: obj\Debug\acw-map.txt
          AndroidConversionFlagFile: obj\Debug\R.cs.flag
          AndroidConversionFlagFile modified: 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\main.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\layout\test.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM
          Skipping: obj\Debug\res\values\strings.xml  7/9/2018 7:57:53 PM <= 7/9/2018 7:57:56 PM

Since `obj\Debug\R.cs.flag` is newer than these files, `IMALLCAPS` is
not converted to lower case!

It appears that the `CopyIfChanged` MSBuild task was not setting the
timestamps of files it copies...

After fixing this, `CopyIfChanged` does not ovewrite `test.xml` in the
first place:

    CopyIfChanged Task
      ...
      Skipping Resources\drawable-hdpi\Icon.png its up to date
      Skipping Resources\drawable-mdpi\Icon.png its up to date
      Skipping Resources\drawable-xhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxhdpi\Icon.png its up to date
      Skipping Resources\drawable-xxxhdpi\Icon.png its up to date
      Skipping Resources\drawable\IMALLCAPS.png its up to date
      Skipping Resources\layout\Main.axml its up to date
      Skipping Resources\layout\test.axml its up to date
      Skipping Resources\values\Strings.xml its up to date
    ModifiedFiles:

Note that `ModifiedFiles` is blank, and my unit test now passes.
jonpryor pushed a commit that referenced this pull request Aug 3, 2018
Fixes: #1958
Context: #1933
Context: #1943

One of the issues I noticed while debugging the issue with #1933 is
that `$(IntermediateOutputPath)build.props` gets invalidated if
`$(DesignTimeBuild)` changes. `build.props` triggers alot of targets
to build completely again, so this is pretty bad for our build times
in an IDE...

Design-Time Builds can run quite frequently in VS Windows, and we
don't want to rebuild a bunch of things unnecessarily when the user
switches back to a regular build.

So a solution, is to use a `obj/Debug/designtime/build.props` that
works independantly of `build.props`. This prevents some slower
targets from running when they shouldn't, such as
`_UpdateAndroidResgen`.

I added a test to validate these changes, which also verify that
`IncrementalClean` isn't deleting these files.

It also is important to point out that any files in the `designtime`
directory are not deleted during a `Clean`. Context on this here:
34a3774

Other changes:
- Updated `.gitignore` for `*.binlog`
- Renamed `_AndroidDesignTimeResDirIntermediate` to
  `_AndroidIntermediateDesignTimeBuildDirectory` and declared it
  earlier in `Xamarin.Android.Common.targets`
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DeleteBinObj issue with Resources
2 participants