Skip to content

Conversation

@nohwnd
Copy link
Member

@nohwnd nohwnd commented Sep 16, 2020

Description

Based on where we are running from, choose the bitness for dotnet SDK either from the OS or from the process. On 64-bit OS when we run from .NET Framework console we want to find 64-bit dotnet SDK, but when running from 32-bit dotnet we want to prefer 32-bit SDK.

Related issue

Fix dotnet/sdk#13157

@nohwnd
Copy link
Member Author

nohwnd commented Sep 16, 2020

@tannergooding in case you want to have a look.

@tannergooding
Copy link
Member

tannergooding commented Sep 16, 2020

👍. Should there also be an explicit switch (probably tracked via a separate issue/PR) to allow specifying the target architecture so devs can force running 32-bit tests from the 64-bit SDK or vice-versa?

@nohwnd nohwnd merged commit f2f8d72 into microsoft:master Sep 17, 2020
@tannergooding
Copy link
Member

@nohwnd, I can log an issue suggesting the above. Should it be logged here in microsoft/vstest or against dotnet/sdk?

@nohwnd
Copy link
Member Author

nohwnd commented Sep 18, 2020

@nohwnd, I can log an issue suggesting the above. Should it be logged here in microsoft/vstest or against dotnet/sdk?

make it vstest please, we will do the work here and in dotnet/sdk if needed as well.

@tannergooding
Copy link
Member

I logged #2576

nohwnd added a commit to nohwnd/vstest that referenced this pull request Sep 24, 2020
nohwnd added a commit that referenced this pull request Sep 24, 2020
* Avoid logging >Task returned false but did not log an error.< (#2557)

* Avoid logging >Task returned false but did not log an error.< on test failure

* Add VSTEST_BUILD_DEBUG env var

* Using namespaces

* Invert the switch because it will be still backwards in the final release

* Use bitness from process or OS (#2571)

* Do not force .NET4.5 in case legacy test settings are provided (#2545)

* Do not force .NET4.5 in case legacy test settings are provided

* Net core app

* Fix runconfig

* Default platform
nohwnd added a commit that referenced this pull request Oct 6, 2020
* Hide -release in console

* Add param block

* Match on whole branch name

* Set var

* Change assertion

* Trim version

* Update dependencies from https://github.com/dotnet/arcade build 20200602.3 (#2456)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 5.0.0-beta.20052.1 -> To Version 1.0.0-beta.20302.3

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update feeds

* Revert to previous dotnet version

* Added new exception handling (#2461)

* Test space added

* Exception handler was added to catch AccessDeniedException while trying to create TestResults folder

* Remove unnecessary space

* Deleted unnecessary test. Changed console message and corrected folder path in that message

* Remove unnecessary dot

* Removed unnecessary lines and usings and coreccted exception message

* Removed unnecessary line

* Updating resource files

* New exception handling was added

* Formatted exception message

* Adding test run attachments processing (#2463)

* v1

* Merging v1

* Rename to MultiTestRunsFinalization

* New version

* More changes

* More changes

* Next changes

* Fix

* test

* More changes

* Dmc chagnes

* next

* small changes

* compiled

* More changes

* acceptance tests green

* Review comments #1

* Resolving more comments

* Tests for design mode client

* Tests for events handler

* revert not related changes

* More changes

* Compiling OK, tests OK

* Unit tests for manager

* More changes

* More tests

* tests for reqeust sender

* more tests

* Tests for cancelling

* Acceptance tests done

* Remove not used stuff

* Fix comments

* Fix race condition in test

* Fix another race condition

* Fix converting to xml

* fix next test

* fix test

* Next changes

* Review changes #1

* Fixing multi test finalization manager tests

* Fixes

* Fix last unit test

* Fix acceptance tests

* Progress feature, compiling + unit tests

* acceptance tests changes

* More changes

* Fixing resources accesability

* Fix test

* Fix race conditions in acceptance tests

* RFC changes merged

* Log warning in case of unexpected message id

* Fix spelling

* Additional comment

* Restore some stuff in interfaces

* Big renaming

* Added processingSettings

* Fix naming

* Move explanation to <remarks>

* Add environment variables to enable MacOS dump

* Fixed code coverage compatibility issue (#2527)

Fixed code coverage compatibility issue

* Print version of the product in log (#2535)

* Trigger dumps asynchronously (#2533)

* Run each dump in a task in netclient dumper

* More reasonable timeout

* Revert "Trigger dumps asynchronously (#2533)" (#2541)

This reverts commit 3454261.

* Remove env variables

* Remove sleeps and extra process dumps from blame

* Fix blame parameter, warning, and add all testhosts to be ngend (#2579)

* Forward merge fixes from master to rc2 (#2581)

* Avoid logging >Task returned false but did not log an error.< (#2557)

* Avoid logging >Task returned false but did not log an error.< on test failure

* Add VSTEST_BUILD_DEBUG env var

* Using namespaces

* Invert the switch because it will be still backwards in the final release

* Use bitness from process or OS (#2571)

* Do not force .NET4.5 in case legacy test settings are provided (#2545)

* Do not force .NET4.5 in case legacy test settings are provided

* Net core app

* Fix runconfig

* Default platform

* Generate release notes in pipeline

* Fix release-notes-script

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Sanan Yuzbashiyev <[email protected]>
Co-authored-by: Jakub Chocholowicz <[email protected]>
Co-authored-by: Codrin-Victor Poienaru <[email protected]>
ghost pushed a commit that referenced this pull request Nov 20, 2020
* Hide -release in console

* Add param block

* Match on whole branch name

* Set var

* Change assertion

* Trim version

* Update dependencies from https://github.com/dotnet/arcade build 20200602.3 (#2456)

Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Feed , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.SignTool , Microsoft.DotNet.SwaggerGenerator.MSBuild
 From Version 5.0.0-beta.20052.1 -> To Version 1.0.0-beta.20302.3

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update feeds

* Revert to previous dotnet version

* Added new exception handling (#2461)

* Test space added

* Exception handler was added to catch AccessDeniedException while trying to create TestResults folder

* Remove unnecessary space

* Deleted unnecessary test. Changed console message and corrected folder path in that message

* Remove unnecessary dot

* Removed unnecessary lines and usings and coreccted exception message

* Removed unnecessary line

* Updating resource files

* New exception handling was added

* Formatted exception message

* Adding test run attachments processing (#2463)

* v1

* Merging v1

* Rename to MultiTestRunsFinalization

* New version

* More changes

* More changes

* Next changes

* Fix

* test

* More changes

* Dmc chagnes

* next

* small changes

* compiled

* More changes

* acceptance tests green

* Review comments #1

* Resolving more comments

* Tests for design mode client

* Tests for events handler

* revert not related changes

* More changes

* Compiling OK, tests OK

* Unit tests for manager

* More changes

* More tests

* tests for reqeust sender

* more tests

* Tests for cancelling

* Acceptance tests done

* Remove not used stuff

* Fix comments

* Fix race condition in test

* Fix another race condition

* Fix converting to xml

* fix next test

* fix test

* Next changes

* Review changes #1

* Fixing multi test finalization manager tests

* Fixes

* Fix last unit test

* Fix acceptance tests

* Progress feature, compiling + unit tests

* acceptance tests changes

* More changes

* Fixing resources accesability

* Fix test

* Fix race conditions in acceptance tests

* RFC changes merged

* Log warning in case of unexpected message id

* Fix spelling

* Additional comment

* Restore some stuff in interfaces

* Big renaming

* Added processingSettings

* Fix naming

* Move explanation to <remarks>

* Add environment variables to enable MacOS dump

* Fixed code coverage compatibility issue (#2527)

Fixed code coverage compatibility issue

* Print version of the product in log (#2535)

* Trigger dumps asynchronously (#2533)

* Run each dump in a task in netclient dumper

* More reasonable timeout

* Revert "Trigger dumps asynchronously (#2533)" (#2541)

This reverts commit 3454261.

* Remove env variables

* Remove sleeps and extra process dumps from blame

* Fix blame parameter, warning, and add all testhosts to be ngend (#2579)

* Forward merge fixes from master to rc2 (#2581)

* Avoid logging >Task returned false but did not log an error.< (#2557)

* Avoid logging >Task returned false but did not log an error.< on test failure

* Add VSTEST_BUILD_DEBUG env var

* Using namespaces

* Invert the switch because it will be still backwards in the final release

* Use bitness from process or OS (#2571)

* Do not force .NET4.5 in case legacy test settings are provided (#2545)

* Do not force .NET4.5 in case legacy test settings are provided

* Net core app

* Fix runconfig

* Default platform

* Generate release notes in pipeline

* Fix the initial assets location of VSTest assets (#2589)

They are being manually pushed to the dotnet-tools feed, not dotnet-core.

* Signing instructions for Newtonsoft.Json.dll added (#2601) (#2603)

* Signing instructions for Newtonsoft.Json.dll added
* Added 3rdParty signature thumbprint to the accept list.

* Cherry-picked signing fixes from `master` (#2619)

* Fix collect dump always

* Fix acceptance tests

* botched merge

* Add regular expressions dependency

* Remove regex

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Sanan Yuzbashiyev <[email protected]>
Co-authored-by: Jakub Chocholowicz <[email protected]>
Co-authored-by: Codrin-Victor Poienaru <[email protected]>
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Medeni Baykal <[email protected]>
jonpryor added a commit to jonpryor/java.interop that referenced this pull request Mar 12, 2021
Context: https://discord.com/channels/732297728826277939/732297952676020316/819978878948868166
Context: microsoft/vstest#2571

Looks like an updated version of `dotnet test` *might* fix my
"test architecture" conundrum, so let's try using .NET 5.0.103!
jonpryor added a commit to dotnet/java-interop that referenced this pull request Mar 17, 2021
Context: 5c756b1
Context: 08ff4db

Context: https://discord.com/channels/732297728826277939/732297952676020316/819978878948868166
Context: microsoft/vstest#2571
Context: dotnet/msbuild#539 (comment)
Context: #816 (comment)

With the `cmake`-based build system knowledge behind 5c756b1 in
mind, update `src/java-interop` to *also* use a `cmake`-based build
system, then add support so it can build on Windows.

On Windows, this builds both e.g.
a 32-bit `bin\Debug\win-x86\java-interop.dll`
*and* 64-bit `bin\Debug\win-x64\java-interop.dll` libraries.

This allows the `Java.Interop-Tests.dll` unit tests to *also* be run
on Windows, under .NET Core [^0].

Note that this requires updating `java_interop_lib_load()` to also
include [`LOAD_LIBRARY_SEARCH_SYSTEM32`][0]:

  * `LOAD_LIBRARY_SEARCH_SYSTEM32`: `%windows%\system32` is searched
    for the DLL and its dependencies.

This specifically is needed so that `JVM.DLL` dependencies such as
`KERNEL32.DLL` can be resolved, allowing `JVM.DLL` to be loaded.

This allows .NET Core on Windows to run `Java.Interop-Tests.dll`!

	> dotnet test bin\TestDebug-netcoreapp3.1\Java.Interop-Tests.dll
	…
	A total of 1 test files matched the specified pattern.
	  Skipped Dispose_ClearsLocalReferences [11 ms]

	Passed!  - Failed:     0, Passed:   617, Skipped:     1, Total:   618, Duration: 2 s - Java.Interop-Tests.dll (netcoreapp3.1)

Install .NET 5.0.103, as that version is required in order for
`dotnet test Java.Interop-Tests.dll` to reliably use a 64-bit process.

~~~

Aside: So you want to create a file which is built into a
*subdirectory* of `$(OutputPath)`.  The "obvious thing" fails:

	<!-- Fails -->
	<ItemGroup>
	  <None Include="$(OutputPath)win-x64\java-interop.dll" />
	</ItemGroup>

This results in:

	error MSB3030: Could not copy the file…

This can be done by:

 1. Providing `%(Link)` item metadata which contains the relative
    directory and filename, *not* `$(OutputPath)`, and

 2. Using `\` *everywhere*.

Thus:

	<!-- Works -->
	<ItemGroup>
	  <None Include="$(OutputPath)win-x64\java-interop.dll">
	    <Link>win-x64\java-interop.dll</Link>
	  </None>
	</ItemGroup>

This works because:

 1. The [`<AppendTargetPath/> task`][1] uses `%(Link)` as an
    *override* when setting `%(TargetPath)`.

 2. The [`_CopyOutOfDateSourceItemsToOutputDirectory` target][2] uses
    `$(OutDir)%(TargetPath)` for `Copy.DestinationFiles`..

 3. The [`<Copy/>` task][3] *ignores errors* when `SourceFiles[i]` and
    `DestinationFiles[i]` are *identical*.  This is why `\` must be
    used, to ensure that the values are identical.

[^0]: …but not .NET Framework, as .NET Framework on CI is launching a
      32-bit process for the unit tests, while we need a 64-bit
      process to match the `JVM.DLL` we're loading.

[0]: https://docs.microsoft.com/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexw
[1]: https://github.com/dotnet/msbuild/blob/17677364d656a98ce4c6eff73b49eddf24f0fd72/src/Tasks/AssignTargetPath.cs#L74
[2]: https://github.com/dotnet/msbuild/blob/17677364d656a98ce4c6eff73b49eddf24f0fd72/src/Tasks/Microsoft.Common.CurrentVersion.targets#L4965-L4977
[3]: https://github.com/dotnet/msbuild/blob/17677364d656a98ce4c6eff73b49eddf24f0fd72/src/Tasks/Copy.cs#L787-L796
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants