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

Initial .NET 8 upgrade #2789

Merged
merged 13 commits into from
Nov 23, 2023
Merged

Initial .NET 8 upgrade #2789

merged 13 commits into from
Nov 23, 2023

Conversation

oskardudycz
Copy link
Collaborator

@oskardudycz oskardudycz commented Nov 21, 2023

PR contains the minimum set of .NET 8 related changes:

  1. Added .NET 8 targets to projects.
  2. Bumped packages to major versions.
  3. Reconfigured CI to Run .NET 8 and supported PostgreSQL versions:
CI .NET Postgres plv8 Serializer
GitHub Actions 6 12.8 STJ
GitHub Actions 6 15-alpine Newtonsoft
GitHub Actions 7 12.8 JSON.NET
GitHub Actions 7 latest STJ
Azure Pipelines 8 12.8 JSON.NET
Azure Pipelines 8 12.8 STJ
Azure Pipelines 8 15-alpine STJ
Azure Pipelines 8 latest Newtonsoft

Removed also benchmark pipeline, as it was unused.
4. Added customisation for docker-compose and tests to allow easier using different PostgreSQL versions by running:

POSTGRES_IMAGE=postgres:15.3-alpine docker compose up
  1. Adjusted code to recent changes in Npgsql:
  • Fixed Event Metadata handling - added the specific frame generation for Dictionary<string, object> to use serializer instead of the build-int Npgsql transformation to dynamic
  • Fixed error mapping for DateTime-related exceptions - the error messages that we're using have changed.
  1. Updated documentation adding PostgreSQL version support policy.
  2. Ignored Can_query_by_hashset_contains test, as @jeremydmiller handled that in the Linq branch.
  3. Fixed ordering tests to have test data not vulnerable for different collation settings.

<Description>Helpers for Marten-backed AspNetCore applications</Description>
<VersionPrefix>6.4.0</VersionPrefix>
<VersionPrefix>7.0.0-alpha.1</VersionPrefix>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using a Directory.Build.props where such common repeated properties can be defined once and are then re-used by MsBuild automatically?

Sou would just need to touch ~5 files instead of the 45 right now.

Copy link
Member

@mysticmind mysticmind Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gfoidl All the primary Marten libraries all use Directory.Build.props. Only the plugins related csproj have specific versions set since they can have their own set of changes and related releases. It is definitely a handful and not 45 is being touched :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just wondering to see the same / similar change to many csproj-files.
So IMO Directory.Build.props could have more common properties, and together with Central Package Management such repeated changes could be minimized.

So nothing urgent or that like, just to save some work for @oskardudycz 😉.

can have their own set of changes and related releases

Ah, OK. TIL they aren't versioned all together.

Copy link
Member

@mysticmind mysticmind Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that when we do a major version update, we set a starting version for these plugin/additional projects in alignment with the major version, reflecting possible breaking changes, dependencies changed etc. After this point, as I said, it can go through it own life cycle of changes/releases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't question that (was so in the first comment, but you told me the versioning strategy then).

So my point is*:

  • set the target-frameworks in the Directory.Build.props, so an update goes only there and doesn't need to be done in all projects (each project still has to specify the property, but can use the variable defined in the Directory.Build.props)
  • use central package management, so updates to packages go to one (central) place and are not spread to all csproj-files

* this comment-thread is now in the wrong place, as the VersionSuffix is a you mention independ for each project

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gfoidl, thank you for suggestions. I'll check the recent improvements around Central Package Management. The biggest challenge is that we try to minimise the project dependencies, and sometimes, we also need to have different package versions for different .NET versions. I'll check how much we can benefit from it. Good call. 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience CPM kind of sucked in pre .NET 8 and had many troubles. Starting with .NET 8 (actually already starting with the previews) it's really good to use and doesn't have any problems.

For me the only point to keep in mind is that PrivateAssets, etc. still needs to be set in the individual csproj that use such a package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that instability was stopping me from trying that further, but thanks for letting me know that improved. I'll give it a try in this PR or (probably) a dedicated one 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gfoidl I added a dedicated issue for that: #2804. I'll try to tackle that in the follow up PR.

@oskardudycz oskardudycz changed the title .Net8 upgrade Initial set of .NET 8 upgrade code Nov 23, 2023
@oskardudycz oskardudycz changed the title Initial set of .NET 8 upgrade code Initial .NET 8 upgrade Nov 23, 2023
@oskardudycz oskardudycz marked this pull request as ready for review November 23, 2023 13:00
@oskardudycz oskardudycz added breaking change enhancement dependencies Pull requests that update a dependency file labels Nov 23, 2023
@oskardudycz oskardudycz self-assigned this Nov 23, 2023
- Bumped packages by major versions
- Adjusted pipeline configurations. Azure DevOps will run .NET 6, GH Actions .NET 8 and 7
Added the specific frame generation for Dictionary<string, object> to use serializer instead of the build-int Npgsql transformation to dynamic
…ions

- Fixed .NET 8,
- Added permutations of supporting PG version: 12 (with plv8), 15 and latest (currently 16),
- Updated pipeline to run plv8 tests only for database with extension configured
- Removed benchmark config that we didn't use at all in recent years
…version

Configured following configurations:
- PG 12.8, .NET 7, plv8, Json.NET
- PG 12.8, .NET 6, plv8, System.Text.Json
- PG 15, .NET 6, JSON.NET
- latest (currently PG 16), .NET 7, System.Text.JSON
@oskardudycz oskardudycz merged commit e9bea56 into master Nov 23, 2023
11 checks passed
@oskardudycz oskardudycz deleted the net8 branch November 23, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x breaking change dependencies Pull requests that update a dependency file enhancement .NET Pull requests that update .net code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants