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

Update TargetFrameworks #8052

Merged
merged 5 commits into from
Mar 18, 2024
Merged

Update TargetFrameworks #8052

merged 5 commits into from
Mar 18, 2024

Conversation

gpetrou
Copy link
Contributor

@gpetrou gpetrou commented Mar 7, 2024

Resolves #8050.

Copy link

cla-checker-service bot commented Mar 7, 2024

💚 CLA has been signed

@gpetrou
Copy link
Contributor Author

gpetrou commented Mar 7, 2024

I used nuget restore -ForceEvaluate to update the packages.lock.json files.

@flobernd flobernd added enhancement 8.x Relates to 8.x client version labels Mar 7, 2024
@flobernd
Copy link
Member

flobernd commented Mar 7, 2024

Seems like the "Unified Release / Assemble" workflow is failing because of the Docker container using the wrong .NET SDK version.

@flobernd
Copy link
Member

flobernd commented Mar 8, 2024

Thanks for updating!

I think there is/was a bug in the way we pass the argument in the Docker commandline:

--env "DOTNET_VERSION" \

@flobernd
Copy link
Member

Thanks again for updating. Seems like we are going down a rabbithole... The newer SDKs seem to use different temp paths which causes building the docker image to fail.

@gpetrou gpetrou force-pushed the TargetFrameworks branch 2 times, most recently from 73dec75 to be67070 Compare March 12, 2024 18:58
@gpetrou
Copy link
Contributor Author

gpetrou commented Mar 13, 2024

The newer SDKs seem to use different temp paths which causes building the docker image to fail.

I think we need to append the user name at the end of the NuGetScrarch folder name. If I understand correctly, this should be user.
https://learn.microsoft.com/en-us/nuget/consume-packages/managing-the-global-packages-and-cache-folders

@flobernd
Copy link
Member

flobernd commented Mar 13, 2024

Maybe we can just set the NUGET_SCRATCH environment variable to /tmp/NuGetScratch to restore the old behavior. Inside the Docker container we only have a single active user anyways.

And we should probably fix the Docker build in a proper way by adding:

--build-arg DOTNET_VERSION="$DOTNET_VERSION"

here:

@flobernd
Copy link
Member

Thanks again for updating!

Seems like the container contains a newer version of Git and we now have to disable the repository owner checks.. Adding:

RUN git config --global --add safe.directory '*'

before:

RUN git rev-parse HEAD .

should probably do the trick.

@flobernd
Copy link
Member

Ok, so regenerating the lockfiles for scripts.fs should be the only thing left to do.

@flobernd
Copy link
Member

Looking good now! Thanks again for the PR and the updates. I'll merge on Monday and include the changes in the next patch release.

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you 🙂

@flobernd flobernd merged commit 4bcd94c into elastic:main Mar 18, 2024
16 of 17 checks passed
@gpetrou gpetrou deleted the TargetFrameworks branch March 19, 2024 05:06
@landlord-matt
Copy link

landlord-matt commented Mar 21, 2024

@flobernd Both .NET 7 and .NET 8 focused on JSON serialisation performance. Did you notice any difference from this .NET 6 version?

Will the patch release also target Elasticsearch 8.6? When do you expect the patch release?

@flobernd
Copy link
Member

@landlord-matt I don't think there will be any noticeable performance benefits as most of the optimizations are related to the codegen features which we don't use yet (they require code changes).

The patch will only target 8.12, but you should be able to update your 8.6 library to 8.12.1 when it's released. I think we had no major breaking changes since then.

A patch release will happen until end of March.

@flobernd flobernd added v8.12.1 and removed v8.13.0 labels Mar 25, 2024
flobernd pushed a commit that referenced this pull request Mar 25, 2024
flobernd pushed a commit that referenced this pull request Mar 25, 2024
flobernd pushed a commit that referenced this pull request Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Relates to 8.x client version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove netcoreapp3.1 and net5.0 from TargetFrameworks and add net8.0
3 participants