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

[vswhere] set WorkingDirectory #2911

Merged

Conversation

jonathanpeppers
Copy link
Member

Context: http://build.devdiv.io/2547192
Context: https://stackoverflow.com/questions/990562/win32exception-the-directory-name-is-invalid

We seem to have hit an odd failure during the MSBuild tests:

Unhandled Exception: System.ComponentModel.Win32Exception: The directory name is invalid
    at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
    at System.Diagnostics.Process.Start()
    at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
    at Xamarin.Android.Tools.VSWhere.MSBuildLocator.Exec(String fileName, String args) in E:\A\_work\1404\s\tools\vswhere\MSBuildLocator.cs:line 55
    at Xamarin.Android.Tools.VSWhere.MSBuildLocator.QueryLatest() in E:\A\_work\1404\s\tools\vswhere\MSBuildLocator.cs:line 28
    at Xamarin.Android.Build.XABuildPaths..ctor() in E:\A\_work\1404\s\tools\xabuild\XABuildPaths.cs:line 124
    at Xamarin.Android.Build.XABuild.Main() in E:\A\_work\1404\s\tools\xabuild\XABuild.cs:line 17

From looking up other examples of this error on the web, it seems that
a weird WorkingDirectory from the build environment would cause
this? WorkingDirectory was not set at all, so we would pickup
whatever the default is.

Let's just set ProcessStartInfo.WorkingDirectory to avoid a
potential problem here.

Context: http://build.devdiv.io/2547192
Context: https://stackoverflow.com/questions/990562/win32exception-the-directory-name-is-invalid

We seem to have hit an odd failure during the MSBuild tests:

    Unhandled Exception: System.ComponentModel.Win32Exception: The directory name is invalid
        at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
        at System.Diagnostics.Process.Start()
        at System.Diagnostics.Process.Start(ProcessStartInfo startInfo)
        at Xamarin.Android.Tools.VSWhere.MSBuildLocator.Exec(String fileName, String args) in E:\A\_work\1404\s\tools\vswhere\MSBuildLocator.cs:line 55
        at Xamarin.Android.Tools.VSWhere.MSBuildLocator.QueryLatest() in E:\A\_work\1404\s\tools\vswhere\MSBuildLocator.cs:line 28
        at Xamarin.Android.Build.XABuildPaths..ctor() in E:\A\_work\1404\s\tools\xabuild\XABuildPaths.cs:line 124
        at Xamarin.Android.Build.XABuild.Main() in E:\A\_work\1404\s\tools\xabuild\XABuild.cs:line 17

From looking up other examples of this error on the web, it seems that
a weird `WorkingDirectory` from the build environment would cause
this? `WorkingDirectory` was not set at all, so we would pickup
whatever the default is.

Let's just set `ProcessStartInfo.WorkingDirectory` to avoid a
potential problem here.
@jonathanpeppers
Copy link
Member Author

Windows test run looks good, 1 test failure.

There does look like there is another random aapt2 failure:

E:\A\_work\1214\s\bin\Release\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Aapt2.targets(146,3): Value cannot be null.
Parameter name: path
   at System.IO.File.Delete(String path)
   at Xamarin.Android.Tasks.Aapt2Link.DoExecute()
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.Execute() [E:\A\_work\1214\s\bin\TestRelease\temp\CompileBeforeUpgradingNuGet\UnnamedProject.csproj]

I will see if I can figure out what is wrong here--we should fix it.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Apr 1, 2019
Context: http://build.devdiv.io/2547670
Context: dotnet#2911 (comment)

We have been occasionally hitting this on CI:

    Xamarin.Android.Aapt2.targets(146,3): Value cannot be null.
    Parameter name: path
        at System.IO.File.Delete(String path)
        at Xamarin.Android.Tasks.Aapt2Link.DoExecute()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.Execute() [E:\A\_work\1214\s\bin\TestRelease\temp\CompileBeforeUpgradingNuGet\UnnamedProject.csproj]

What I think is happening:

* We call `tempFiles.Add()` during a `Parallel.ForEach`. Bad...
* Instead of an exception happening during `Add()`, a `null` is
  somehow added to the underlying array--which is corrupted.

Instead I think we should add locking around all usage of this
`tempFiles` member variable. I looked at `ConcurrentBag<T>`, but it
seemed more appropriate to use a lock because of the `foreach` +
`Clear()` calls.
dellis1972 pushed a commit that referenced this pull request Apr 2, 2019
Context: http://build.devdiv.io/2547670
Context: #2911 (comment)

We have been occasionally hitting this on CI:

    Xamarin.Android.Aapt2.targets(146,3): Value cannot be null.
    Parameter name: path
        at System.IO.File.Delete(String path)
        at Xamarin.Android.Tasks.Aapt2Link.DoExecute()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.Execute() [E:\A\_work\1214\s\bin\TestRelease\temp\CompileBeforeUpgradingNuGet\UnnamedProject.csproj]

What I think is happening:

* We call `tempFiles.Add()` during a `Parallel.ForEach`. Bad...
* Instead of an exception happening during `Add()`, a `null` is
  somehow added to the underlying array--which is corrupted.

Instead I think we should add locking around all usage of this
`tempFiles` member variable. I looked at `ConcurrentBag<T>`, but it
seemed more appropriate to use a lock because of the `foreach` +
`Clear()` calls.
@jonathanpeppers jonathanpeppers merged commit 6a9a882 into dotnet:master Apr 2, 2019
@jonathanpeppers jonathanpeppers deleted the vswhere-workingdirectory branch April 2, 2019 13:35
jonpryor pushed a commit that referenced this pull request Apr 3, 2019
Context: http://build.devdiv.io/2547670
Context: #2911 (comment)

We have been occasionally hitting this on CI:

    Xamarin.Android.Aapt2.targets(146,3): Value cannot be null.
    Parameter name: path
        at System.IO.File.Delete(String path)
        at Xamarin.Android.Tasks.Aapt2Link.DoExecute()
        at System.Threading.Tasks.Task.InnerInvoke()
        at System.Threading.Tasks.Task.Execute() [E:\A\_work\1214\s\bin\TestRelease\temp\CompileBeforeUpgradingNuGet\UnnamedProject.csproj]

What I think is happening:

* We call `tempFiles.Add()` during a `Parallel.ForEach`. Bad...
* Instead of an exception happening during `Add()`, a `null` is
  somehow added to the underlying array--which is corrupted.

Instead I think we should add locking around all usage of this
`tempFiles` member variable. I looked at `ConcurrentBag<T>`, but it
seemed more appropriate to use a lock because of the `foreach` +
`Clear()` calls.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 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.

2 participants