-
Notifications
You must be signed in to change notification settings - Fork 136
Add CLI to known-good #418
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
Conversation
NuGet.Configuration isn't in the blob feed, which would explain all these. Either it wasn't built, or it isn't copied to the blob feed correctly. Looking into it. |
Enabling the NuGet.Configuration package build worked, next up:
I'm going to try adding a package reference in CLI to force the source-built versions to be used. |
Looking again, that's an internet version, not source-built--I think these packages aren't being produced by CoreFX when they should be. |
Wes pointed me to #210, which tracks adding the type of CoreFX build that produces those packages. (Apparently not trivial.) Tomas pointed me to some commits I can revert to bring symreader and symreader-portable back to depending on 1.3.1 and 1.4.2, so the mismatch will go away. I'm going to add patches to revert: dotnet/symreader@ac20b8b dotnet/symreader-portable@58d468a |
Info on the MSBuild fix this PR needed: the source-built Microsoft.Build.Runtime package didn't mark the included |
The last commit has successful first-stage CI legs, but @dseefeld locally saw the tarball fail in MSBuild because I had the MSBuild commit change without the (I forgot to change the fork back to |
@dotnet-bot test ci please |
@dotnet-bot test ci please |
Fix cli/run-build.sh's argsnotargets. Fix CLI's NuGet.Config generation with source-build directory inputs. Disable crossgen until we figure out the dll load error. Disable bundled tools until we figure out the best solution for dotnet-dev-certs, dotnet-ef, etc. Use existing GitInfo* properties if provided.
|
||
if [ $BUILD -eq 1 ]; then | ||
- (set -x; dotnet msbuild build.proj /bl:msbuild.generatepropsfile.binlog /p:Architecture=$ARCHITECTURE $CUSTOM_BUILD_ARGS /p:GeneratePropsFile=true /t:WriteDynamicPropsToStaticPropsFiles $argsnotargets) | ||
+ (set -x; dotnet msbuild build.proj /bl:msbuild.generatepropsfile.binlog /p:Architecture=$ARCHITECTURE $CUSTOM_BUILD_ARGS /p:GeneratePropsFile=true /t:WriteDynamicPropsToStaticPropsFiles ${argsnotargets[@]}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to guess the intention of using $argsnotargets
here is to pass the entire array through.
Perhaps this is meant to be $(IFS=' '; echo "${argsnotargets[*]}")
?
@johnbeisner is that right? This is a patch in source-build that we'll eventually take into CLI, so I'd like to get it right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would make sense to me. I borrowed the syntax I used from for arg in ${args[@]}
. (That for loop strongly indicates to me that all the non-targets args are meant to be passed, but this usage was bugged.)
@@ -0,0 +1,100 @@ | |||
From cd96ec40e18bf5cdce6cb3aad8b699051da7bf34 Mon Sep 17 00:00:00 2001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also patch this:
private bool ShouldGenerateAspNetCertificate()
{
#if DOTNET_BUILD_FROM_SOURCE
return false;
#else
var generateAspNetCertificate =
_environmentProvider.GetEnvironmentVariableAsBool("DOTNET_GENERATE_ASPNET_CERTIFICATE", true);
return ShouldRunFirstRunExperience() &&
generateAspNetCertificate &&
!_aspNetCertificateSentinel.Exists();
#endif
}
to prevent the first run experience from telling users a certificate was created when one wasn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #434 to get this in.
Over than the additional patch, I think this looks good. Looking over the CLI patches, I think I should be able to get those all in upstream. |
standardJobSetup sets the 120-minute default that we need to override.
I queued this unshared RHEL with the extended timeout: https://ci.dot.net/job/dotnet_source-build/job/dev_release_2.1/job/GenPRTest/job/RHEL7.2_Unshared_Release_prtest/6/. (Expect the automatically triggered one to fail due to timeout.) |
OSX failed to "download" a local file:
Building locally for repro. My first thought is that the filename Core-Setup produced could be wrong. Fixed with next commit, see #438. |
This makes the Core-Setup filename match what CLI expects.
Filed https://github.com/dotnet/core-eng/issues/3291 for the CI infra failures on the legs that use Ubuntu. |
@dotnet-bot |
Merging: RHEL7.2 leg is expected to time out, and with everything else green it seems unlikely the OSX change broke it since I tested with the extended timeout on the previous commit. |
No description provided.