Skip to content

[Xamarin.Andorid.Build.Tasks] Check if LogicalName is valid before check Resource File.#3327

Merged
jonathanpeppers merged 2 commits intodotnet:masterfrom
dellis1972:CheckForInvalidResourceFileNamesLogicalName
Jul 9, 2019
Merged

[Xamarin.Andorid.Build.Tasks] Check if LogicalName is valid before check Resource File.#3327
jonathanpeppers merged 2 commits intodotnet:masterfrom
dellis1972:CheckForInvalidResourceFileNamesLogicalName

Conversation

@dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Jul 4, 2019

Some of our customers are generating Resource files
which have invalid file names (as far as aapt/aapt2 is
concerned). They use the LogicalName meta data to
provide the "Real" name that the build process should
use.

However when using aapt2 our CheckForInvalidResourceFileNames
task was NOT using the LogicalName metadata. It was
just using the filename. This breaks their projects with
an

APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.-]+

or

APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.]+

We should really support this scenario since we allow the
use of LogicalName for resource files anyway.

This commit fixes that issue and adds a unit test.

@dellis1972
[Xamarin.Andorid.Build.Tasks] Check if LogicalName is valid before ch…
9111f99 
…eck Resource File.

Some of our customers are generating `Resource` files
which have invalid file names (as far as aapt/aapt2 is
concerned). They use the `LogicalName` meta data to
provide the "Real" name that the build process should
use.

However when using `aapt2` our `CheckForInvalidResourceFileNames`
task was NOT using the `LogicalName` metadata. It was
just using the filename. This breaks their projects with
an

	APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.-]+

or

	APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.]+

We should really support this scenario since we allow the
use of `LogicalName` for resource files anyway.

This commit fixes that issue and adds a unit test.
@dellis1972 dellis1972 added this to the d16-3 milestone Jul 4, 2019
@dellis1972 dellis1972 self-assigned this Jul 4, 2019
@jeromelaban jeromelaban mentioned this pull request Jul 4, 2019
Closed
9 tasks
@jonathanpeppers
Copy link
Member

jonathanpeppers commented Jul 5, 2019

I think there are test failures related to the changes here: https://jenkins.mono-project.com/job/xamarin-android-pr-pipeline-release/1552/testReport/

There is a CheckLogicalNamePathSeperators test that fails with:

"/Users/builder/jenkins/workspace/xamarin-android-pr-pipeline-release_2/xamarin-android/bin/TestRelease/temp/CheckLogicalNamePathSeperators_False/Library1/Library1.csproj" (Build target) (1) ->
(_GenerateAndroidResourceDir target) -> 
  Resources/Test/Test2.png : error APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.]+. [/Users/builder/jenkins/workspace/xamarin-android-pr-pipeline-release_2/xamarin-android/bin/TestRelease/temp/CheckLogicalNamePathSeperators_False/Library1/Library1.csproj]

    0 Warning(s)
    1 Error(s)

@dellis1972
Copy link
Contributor Author

dellis1972 commented Jul 6, 2019

Ok so this is a bit of a problem. CheckLogicalNamePathSeperators suggests that we are fixing up path separators in the LogicalName even though they are invalid. However we also need to be able to support the use of LogicalName. So I guess we need to ignore invalid path separators in the CheckForInvalidResourceFileNames task ...

@ghuntley
Copy link
Member

ghuntley commented Jul 9, 2019

See also:

@dellis1972
Copy link
Contributor Author

dellis1972 commented Jul 9, 2019

@jonathanpeppers this should be good to go now. I added a test. The CI system is being a bit weird though, some of the bots have been waiting for ages.

jonathanpeppers
jonathanpeppers approved these changes Jul 9, 2019
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

The github status checks for Azure DevOps have a bug right now, only the overall status of the Xamarin.Android build is reported correctly right now.

Only a Lint-related test failed on Windows, this one looks good to me now.

@jonathanpeppers jonathanpeppers merged commit e0145d1 into dotnet:master Jul 9, 2019
@dellis1972 dellis1972 deleted the CheckForInvalidResourceFileNamesLogicalName branch July 9, 2019 16:21
@ghuntley ghuntley mentioned this pull request Aug 14, 2019
Closed
9 tasks
jonpryor pushed a commit that referenced this pull request Aug 21, 2019
@dellis1972 @jonpryor
[Xamarin.Andorid.Build.Tasks] Check if LogicalName is valid before ch…
ebabe58 
…eck Resource File. (#3327)

Some of our customers are generating `Resource` files
which have invalid file names (as far as aapt/aapt2 is
concerned). They use the `LogicalName` meta data to
provide the "Real" name that the build process should
use.

However when using `aapt2` our `CheckForInvalidResourceFileNames`
task was NOT using the `LogicalName` metadata. It was
just using the filename. This breaks their projects with
an

	APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.-]+

or

	APT0000: Invalid file name: It must contain only [^a-zA-Z0-9_.]+

We should really support this scenario since we allow the
use of `LogicalName` for resource files anyway.

This commit fixes that issue and adds a unit test.

The `<CheckForInvalidResourceFileNames/>` MSBuild task can also
ignore invalid path separators as they are fixed up
@brendanzagaeski
Copy link
Contributor

brendanzagaeski commented Sep 4, 2019

A new preview version has now been published that includes a fix for this item.

Fix included in Xamarin.Android 10.0.0.40.

Fix included on Windows in Visual Studio 2019 version 16.3 Preview 3. To try the preview version that includes the fix, check for the latest updates in Visual Studio Preview.

Fix included on macOS in Visual Studio 2019 for Mac version 8.3 Preview 3. To try the preview version that includes the fix, check for the latest updates on the Preview updater channel.

@brendanzagaeski
Copy link
Contributor

brendanzagaeski commented Sep 25, 2019

Release status update

A new Release version has now been published that includes the fix for this item.

Fix included in Xamarin.Android 10.0.0.43

Fix included on Windows in Visual Studio 2019 version 16.3. To get the new version that includes the fix, check for the latest updates or install the latest version from https://visualstudio.microsoft.com/downloads/.

Fix included on macOS in Visual Studio 2019 for Mac version 8.3. To get the new version that includes the fix, check for the latest updates on the Stable updater channel.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

Assignees

@dellis1972 dellis1972

Labels

None yet

Projects

None yet

Milestone

Published: 10.0 (d16-3)

Development

Successfully merging this pull request may close these issues.

4 participants

@dellis1972 @jonathanpeppers @ghuntley @brendanzagaeski