[NativeAOT] Fix --make-repro-path ILC option

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013

[ivanpovazan]

This PR fixes a regression with --make-repro-path command line option which was caused by upgrading the System.CommandLine version in: #84229

The issue is related to the fact that CliOption.Name now includes -- (eg: option.Name == --targetos:OSX) causing --make-repro-path to generate extra hyphens in the repro.rsp file, for example:

----reference:0/WindowsBase.dll
----reference:1/System.Private.CoreLib.dll
----reference:2/System.Private.DisabledReflection.dll
...

Tested manually by:

1. Creating a repro.zip by including the --make-repro-path option during compilation
2. Unzipping the repro.zip
3. Compiling the assemblies from the unzipped folder by passing the generated repro.rsp

[ivanpovazan]

/cc: @am11

[am11]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

[am11]

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

[ivanpovazan]

Thank you! In my mind, I was confident that I have tested this scenario when I sent out the PR, but apparently additional hyphens was not something I paid attention to. 🙈

No worries, I accidentally bumped into this. As you rightfully suggested:

@ivanpovazan, not for this PR, but do you think we can create a test for make-repro? e.g. build a command with many options and arguments and run it with make-repro, then extract the repro package, rerun the extracted command and compare results.

cc @adamsitnik

it would be great to have a test, so we catch this kind of unintentional regressions. However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like. Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

[am11]

However, covering all supported switches seems like a moving target, so not exactly sure what would the test look like.

I agree. Perhaps the options which are frequently used by ILC, e.g. from obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp file when we PublishAot a console app.

Additionally, would we just check whether the resulting repro.rsp is as expected, or a full successful compilation of a repro package would have to be used for verification?

I think the original and generated rsp files are not comparable easily. Moreover, if we want to also verify that the repro package actually works, we can do something similar to:

• dotnet publish -p:PublishAot=true a console app to auto-generate the RSP file under obj/ dir (so we don't need to worry about keeping it in sync with moving target)
• Append --make-repro-path line to aot.ilc.rsp
• Execute ilc @"obj/$(Configuration)/$(TFM)/$(RID)/native/aot.ilc.rsp"
• Extract the package in repro1/
• Insert --make-repro-path line in repro1's repro.rsp and run ilc @repro.rsp (working directory needs to be the extraction location)
• Extract the second package in repro2/
• Insert --make-repro-path line in repro2's repro.rsp
• Compare if the extracted RSP files match (produce zero diffs)

[ivanpovazan]

That sounds good, however I am not sure where would such test be placed (we had similar discussions around build/running ios test apps, which ended up being treated as functional tests - sharing the Mono setup).

@MichalStrehovsky what do you think?

[jkotas]

dotnet/runtime repo does not have a good place for tests that exercise E2E tool flows. A good place to add a test like this one would be the SDK repo (see https://github.com/dotnet/sdk/blob/364773edba30d46ac95e3b32c08f46168e96bd35/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs for prior art).

[jkotas]

/backport to release/8.0

[github-actions]

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5952974913

[jkotas]

test

Opened #91013