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

Fallout from Paket and FAKE deletion #663

Closed
6 of 7 tasks
Smaug123 opened this issue Feb 24, 2024 · 19 comments · Fixed by #673
Closed
6 of 7 tasks

Fallout from Paket and FAKE deletion #663

Smaug123 opened this issue Feb 24, 2024 · 19 comments · Fixed by #673

Comments

@Smaug123
Copy link
Contributor

Smaug123 commented Feb 24, 2024

(For after #662 because it will conflict.)

  • It would make sense for appveyor on branches to additionally check that dotnet pack will succeed. This should be a simple matter of adding the dependency from CI on Pack.
  • Fix up the logo. Currently we're using the deprecated PackageLogoUrl. (logo file is here: https://github.com/fscheck/FsCheck/blob/master/docs/img/logo.png)
  • Look at moving to NerdBank.GitVersioning; either way, get rid of the NuGet dep in build.fsx that make the script take multiple seconds to start. (wontfix: Fallout from Paket and FAKE deletion #663 (comment))
  • Stop WatchDocs from printing out empty stdout every 10s
  • Error output from subprocesses is space-separated but it should be newline-separated
  • Delete the commented-out bits of Paket that still litter build.fsx
  • dotnet pack release notes need to be escaped or passed via other means (see latest AppVeyor builds)
@Smaug123 Smaug123 changed the title Run Pack stage in CI Fallout from Paket and FAKE deletion Feb 24, 2024
@Smaug123 Smaug123 mentioned this issue Feb 24, 2024
29 tasks
@kurtschelfthout
Copy link
Member

// I *believe* it is impossible to have a fixed (not floating) version number from a source reference
// via `dotnet pack`. Without this next bit, FsCheck.Xunit vA.B.C depends on >= FsCheck vA.B.C, not
// on FsCheck exactly at vA.B.C.

Have we also lost the NUnit (>= 3.13.1 && < 4.0.0) now? It seems to have turned into NUnit (>= 3.13.1). Is there something in dotnet pack that can specify that like paket.template?

@Smaug123
Copy link
Contributor Author

Smaug123 commented Feb 25, 2024

Are you observing that? I see the correct bound.
Screenshot 2024-02-25 at 10 54 57

And indeed when I unzip the FsCheck.NUnit.nupkg file and look at its nuspec, it correctly says:

<dependency id="FsCheck" version="[3.0.0-rc2]" exclude="Build,Analyzers" />
<dependency id="NUnit" version="[3.13.1, 4.0.0)" exclude="Build,Analyzers" />

@kurtschelfthout
Copy link
Member

Ah, is it the line in Directory.packages.props that does it?

    <PackageVersion Include="NUnit" Version="[3.13.1,4.0.0)" />

That's alirght then.

Push worked btw, I just pushed rc2. But I think I messed up the xunit dependency restriction. Luckily there is no v3 of xunit so no harm no foul, I hope.

@Smaug123
Copy link
Contributor Author

That's the line, yep. You'll want this:

    <PackageVersion Include="xunit.extensibility.execution" Version="[2.4.1, 3.0.0)" />

in Directory.Packages.props.

@kurtschelfthout
Copy link
Member

This always fails with "Forbidden". I've regenerated my token. It has I think the right permissions (see below). I set the token as env variable in exactly the same way as NUGET_KEY, which works.

Clearly either my token is missing some permissions, or it's somehow not being passed correctly. I've checked the header and it looks fine, compared to the docs. https://docs.github.com/en/rest/releases/releases?apiVersion=2022-11-28#create-a-release

Any ideas?

    use client = new HttpClient ()
    client.DefaultRequestHeaders.Add ("Accept", "application/vnd.github+json")
    client.DefaultRequestHeaders.Add ("X-GitHub-Api-Version", "2022-11-28")
    client.DefaultRequestHeaders.Authorization <- AuthenticationHeaderValue ("Bearer", pat)
    let postData = JsonSerializer.Serialize releaseSpec
    use content = new StringContent (postData, Encoding.UTF8, "application/json")
    use response = client.PostAsync($"https://api.github.com/repos/%s{gitOwner}/%s{gitName}/releases", content).Result.EnsureSuccessStatusCode()
    let response = response.Content.ReadAsStringAsync().Result
    let output = JsonSerializer.Deserialize<GitHubReleaseResponse> response

image

@Smaug123
Copy link
Contributor Author

Let me set up a toy repo and try it.

@kurtschelfthout
Copy link
Member

Worked just now with a curl. So my token must be ok, it's just not being set in the header correctly somehow.

@Smaug123
Copy link
Contributor Author

Currently you're setting Authorization twice (client.DefaultRequestHeaders.Add ("Authorization", "2022-11-28")) - that's my bad, you need to delete that line. Haven't worked out if that's the only problem yet.

@kurtschelfthout
Copy link
Member

Yeah, did that, with that it didn't work at all. (See my commit on master just now). After that I get the Forbidden error.

@kurtschelfthout
Copy link
Member

Fixed up the release manually for now, no rush. Stepping away from computer now for a while.

@Smaug123
Copy link
Contributor Author

Smaug123 commented Feb 25, 2024

Got it - I need to learn never to use anything other than SendAsync, because all the helpers are liable to hide crucial information and this catches me out very regularly. The problem is https://docs.github.com/en/rest/using-the-rest-api/getting-started-with-the-rest-api?apiVersion=2022-11-28#user-agent ("you need a user agent").

I've raised the fix as #665 .

@kurtschelfthout
Copy link
Member

Look at moving to NerdBank.GitVersioning;

From a quick look, this doesn't do what we want (i.e. read the release notes and pick up the version from there). It seems to require a separate json file with the verison in it. If that's the case, I'd prefer just keeping the fake dependency for this, I don't think it'll get in the way much.

@Smaug123
Copy link
Contributor Author

Fair enough!

@Smaug123
Copy link
Contributor Author

Smaug123 commented Feb 25, 2024

Oh nooooo, it looks like dotnet pack interprets commas in -p:PackageReleaseNotes=:

The failed process call (note that this is almost all a command line):

Process 'dotnet' failed with nonzero exit code 1. Args: pack --configuration Release -p:Version=3.0.0-rc2 --output bin -p:PackageReleaseNotes=Breaking change: confusingly named `StringNoNnulls` is renamed to `StringNoNullChar`.
Breaking change: The operators `|@`, `@|` and `%>` are removed. Please use `Prop.label` instead.
Negative decimals are now also generated. (by Stephen Smith)
Relaxed FsCheck.Xunit's restriction on xUnit versions. (by Tom Rijnbeek)
Made `Gen.choose64` public.
Removed dependency on FAKE and paket in favor of standard .NET tools. (by Patrick Stevens)
Added more `ForAll` overloads for various `Task` types.
The collections types `NonEmptySet`, `NonEmptyArray` and `FixedLengthArray` now implemented `IEnnumerable` to avoid a call to `Get` in common scenarios..

Stdout of dotnet pack:

  MSBuild version 17.8.3+195e7f5a3 for .NET
MSBUILD : error MSB1006: Property is not valid.
Switch:  `@|` and `%>` are removed. Please use `Prop.label` instead.
Negative decimals are now also generated. (by Stephen Smith)
Relaxed FsCheck.Xunit's restriction on xUnit versions. (by Tom Rijnbeek)
Made `Gen.choose64` public.
Removed dependency on FAKE and paket in favor of standard .NET tools. (by Patrick Stevens)
Added more `ForAll` overloads for various `Task` types.
The collections types `NonEmptySet`
For switch syntax, type "MSBuild -help"

@Smaug123
Copy link
Contributor Author

I'll put it in an env var instead.

@kurtschelfthout
Copy link
Member

I can try to avoid commas in the release notes ;)

@Smaug123
Copy link
Contributor Author

On the plus side, Appveyor does now catch that error! https://ci.appveyor.com/project/kurtschelfthout/fscheck/builds/49266851

@Smaug123
Copy link
Contributor Author

Oh my god, MsBuild itself appears to perform XML escaping on the ampersands I am trying to use to perform XML escaping. If we can get XML-escaped lines in there then they will correctly be shown in the release notes:
Screenshot 2024-02-25 at 20 36 41
https://github.com/jstedfast/MimeKit/blob/4315487b9b469efd93d53c998f145b501e6d55f4/nuget/MimeKit.nuspec#L29

But I can't get them in there.

@Smaug123
Copy link
Contributor Author

Gone with some truly amazing hacks (#672)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants