Skip to content
This repository has been archived by the owner on Nov 20, 2023. It is now read-only.

Restore.cmd fails on non-existing flags "-all" and "-nobuild" #360

Merged
merged 1 commit into from
Apr 13, 2020
Merged

Restore.cmd fails on non-existing flags "-all" and "-nobuild" #360

merged 1 commit into from
Apr 13, 2020

Conversation

tebeco
Copy link
Contributor

@tebeco tebeco commented Apr 13, 2020

What

In dotnet/tye the restore.cmd directly call ./eng/common/Build.ps1 (dotnet/arcade)
It does try to call this script with -all and -nobuild
./eng/common/Build.ps1 script does not have sutch switch -all or -nobuild

Why ?

Original error :
image

After removing -all :
image

Investigation

It seems that this logic was added to look like the one on dotnet/aspnetcore, the difference between both repository is that:

  • dotnet/tye directly calls ./eng/common/Build.ps1 (no such switch)
  • dotnet/aspnetcore calls first ./build.ps1 (the switch logic is in there), and later on call ./eng/common/Build.ps1

Side effect (aka: what we "loose" on merge)

-all

TLDR : Probably not important for now in this repo does not have (yet ?) any Node/Java Build to handle

After trying to understand the build.ps1 at root level of dotnet/aspnetcore
here is what the -all does :

  • Set/Add an MsBuild property BuildAllProjects to true
if ($All) {
    $MSBuildArguments += '/p:BuildAllProjects=true'
}
  • Detect if Node/Java are present + Warn/Error
if ($BuildManaged -or ($All -and (-not $NoBuildManaged))) {
    if ((-not $BuildNodeJS) -and (-not $NoBuildNodeJS)) {
        $node = Get-Command node -ErrorAction Ignore -CommandType Application
    }
    ... Logs here
}
...
if (-not $foundJdk -and $RunBuild -and ($All -or $BuildJava) -and -not $NoBuildJava) {
    Write-Error "Could not find the JDK......"
}

-nobuild

$RunBuild = if ($NoBuild) { $false } else { $true }

# Run restore by default unless -NoRestore is set.
# -NoBuild implies -NoRestore, unless -Restore is explicitly set (as in restore.cmd)
$RunRestore = if ($NoRestore) { $false }
    elseif ($Restore) { $true }
    elseif ($NoBuild) { $false }
    else { $true }

# Target selection
$MSBuildArguments += "/p:Restore=$RunRestore"
$MSBuildArguments += "/p:Build=$RunBuild"
if (-not $RunBuild) {
    $MSBuildArguments += "/p:NoBuild=true"
}

@tebeco tebeco changed the title Restore.cmd fails Restore.cmd fails on non-existing flags "-all" and "-nobuild" Apr 13, 2020
@tebeco tebeco changed the title Restore.cmd fails on non-existing flags "-all" and "-nobuild" [WIP] Restore.cmd fails on non-existing flags "-all" and "-nobuild" Apr 13, 2020
@tebeco tebeco changed the title [WIP] Restore.cmd fails on non-existing flags "-all" and "-nobuild" Restore.cmd fails on non-existing flags "-all" and "-nobuild" Apr 13, 2020
@jkotalik
Copy link
Contributor

I think this is fine for now. I doubt we will need these switches for -all.

@jkotalik jkotalik merged commit 8429c36 into dotnet:master Apr 13, 2020
@tebeco
Copy link
Contributor Author

tebeco commented Apr 13, 2020

yeah the -all seems obvious for now
i wonder about the -nobuild though as there many thing i don't know about aspnetcore CI/Build scripts

kishanAnem pushed a commit to kishanAnem/tye that referenced this pull request Apr 19, 2020
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