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

Span compat #757

Merged
merged 49 commits into from
Apr 13, 2023
Merged

Span compat #757

merged 49 commits into from
Apr 13, 2023

Conversation

martijn00
Copy link
Contributor

Add netstandard support for Span on: #754

@ebozduman
Copy link
Collaborator

First, I see the following issue when I run functional tests against 4.8:

/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj : error NU1605: Warning As Error: Detected package downgrade: System.ValueTuple from 4.5.0 to 4.4.0. Reference the package directly from the project to select a different version. 
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj : error NU1605:  Minio.Functional.Tests -> Minio -> System.Text.Json 7.0.2 -> System.ValueTuple (>= 4.5.0) 
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj : error NU1605:  Minio.Functional.Tests -> Minio -> System.ValueTuple (>= 4.4.0)

After fixing it by simply bumping up the version of System.ValueTuple package to 4.5.0, I see the following:

$ dotnet run --project ./Minio.Functional.Tests/Minio.Functional.Tests.csproj
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/RandomStreamGenerator.cs(28,27): error CS1503: Argument 1: cannot convert from 'System.Span<byte>' to 'byte[]' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(720,62): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(721,42): error CS1061: 'FileStream' does not contain a definition for 'DisposeAsync' and no accessible extension method 'DisposeAsync' accepting a first argument of type 'FileStream' could be found (are you missing a using directive or an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(804,62): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(805,42): error CS1061: 'FileStream' does not contain a definition for 'DisposeAsync' and no accessible extension method 'DisposeAsync' accepting a first argument of type 'FileStream' could be found (are you missing a using directive or an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(885,62): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(886,42): error CS1061: 'FileStream' does not contain a definition for 'DisposeAsync' and no accessible extension method 'DisposeAsync' accepting a first argument of type 'FileStream' could be found (are you missing a using directive or an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(926,33): error CS0117: 'File' does not contain a definition for 'ReadAllBytesAsync' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(960,33): error CS0117: 'File' does not contain a definition for 'ReadAllBytesAsync' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(1498,41): error CS0117: 'MD5' does not contain a definition for 'HashData' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(1500,38): error CS0117: 'MD5' does not contain a definition for 'HashData' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(2612,41): error CS1503: Argument 1: cannot convert from 'string' to 'char' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(2697,49): error CS0103: The name 'HttpUtility' does not exist in the current context [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4574,62): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4575,42): error CS1061: 'FileStream' does not contain a definition for 'DisposeAsync' and no accessible extension method 'DisposeAsync' accepting a first argument of type 'FileStream' could be found (are you missing a using directive or an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4717,28): error CS0117: 'File' does not contain a definition for 'WriteAllLinesAsync' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4750,66): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4751,46): error CS1061: 'FileStream' does not contain a definition for 'DisposeAsync' and no accessible extension method 'DisposeAsync' accepting a first argument of type 'FileStream' could be found (are you missing a using directive or an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4758,61): error CS0117: 'File' does not contain a definition for 'ReadAllTextAsync' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4810,35): error CS0117: 'File' does not contain a definition for 'OpenHandle' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4811,57): error CS1503: Argument 2: cannot convert from 'System.IO.FileAccess' to 'System.IO.FileMode' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/FunctionalTest.cs(4827,48): error CS1503: Argument 2: cannot convert from 'System.Threading.CancellationToken' to 'int' [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio.Functional.Tests/Minio.Functional.Tests.csproj]

The build failed. Fix the build errors and run again.

I started looking into these, but then I had to switch to another task. If you'd like I can send you the diffs of the files I've touched.

@martijn00
Copy link
Contributor Author

@ebozduman Why would you switch that to 4.8? It is even on 6.0 on the older branches: https://github.com/minio/minio-dotnet/blob/4.0.7/Minio.Functional.Tests/Minio.Functional.Tests.csproj#L3

It never worked on net4.8. The library will be compatible with 4.8 this way, but the test are just an output, and the code itself should not be compatible.

@martijn00
Copy link
Contributor Author

@ebozduman Anyways, i've added the support for net48 in the tests. Can you get the nuget release out please?

@ebozduman
Copy link
Collaborator

@martijn00,
.NET4.x was supposed to be tested and supported since from the first day. So, we need to fix any incompatibility issues, add required testing in CI process and fully support it.

Thank you for working on this task.

@martijn00
Copy link
Contributor Author

@ebozduman I've just fixed the failing test

@ebozduman
Copy link
Collaborator

@martijn00

I will push some changes in a short while. These changes go on top of your PR, #757, and it completes the changes required to support older 4.x dotnet releases together with 6.0 and 7.0 dotnet releases we support. I ran all functional tests against all the supported releases, 4.7.2, 4.8, 4.8.1, 6.0, 7.0, and they all passed successfully.

However, I expect the dotnet regitlint check to fail during the CI build process because of a bug in JetBrains.ReSharper.Psi.ExtensionsAPI.Tree.ModificationUtil.DeleteChildRange.
I am looking into it and I think there is a workaround to it.

I appreciate if you could review my changes in the meantime.

As soon as the lint check issue is resolved and the review is complete, we can merge PR#757 and start a new release.

@ebozduman
Copy link
Collaborator

@martijn00 ,

Please disregard the last 2 commits. It is my bad (I was sleep deprived). I'll fix it.

@martijn00
Copy link
Contributor Author

@ebozduman hehe alright, I'll wait a bit then because you did some strange things in there. Also I want to say again, lets get a release out first and then follow up with more fixes and warning updates because this is getting way too big.

@ebozduman
Copy link
Collaborator

Yes @martijn00 ,
I agree.
I had to reverse some of your changes to make the code work for 4.x and for core releases.
We can make the code better as soon as a new Minio package is built and released.

@martijn00
Copy link
Contributor Author

@ebozduman what are you doing with those commits? Please revert because it is not correct. Let me know what you want so I can have a look if changes need to be made.

@ebozduman
Copy link
Collaborator

ebozduman commented Apr 4, 2023

You're right. I'll clean it up.
Sorry for the mess.

@ebozduman
Copy link
Collaborator

Done. My recent commit is removed.

As soon as, the build testing is a PASS, we can merge the PR.

Could you please look into the build lint failures?
When the lint issues are addressed, there will be 2 more little fixes we'll need:

1- Func. test ObjectLockConfigurationAsync_Test1 fails

System.InvalidOperationException: There was an error reflecting type
'Minio.DataModel.Tags.Tagging'.
 ---> System.NotSupportedException: Cannot serialize member
Minio.DataModel.Tags.Tagging.Tags of type
System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib,
Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.String,
System.Private.CoreLib, Version=6.0.0.0, Culture=neutral,
PublicKeyToken=7cec85d7bea7798e]], because it implements
 IDictionary.
:
:

2- Mint.Logger.Log() function escapes some chars, special chars if any appears in the object name, function or in any other string in the log message.

{"name":"minio-dotnet : CopyObject_Test4", 
"function":"Task\u003CCopyObjectResult\u003E 
CopyObjectAsync(CopyObjectArgs args, 
CancellationToken cancellationToken = default(CancellationToken))",
"args":{"bucketName":"minio-dotnet-example-caeox1ekrstju5b",
"objectName":"d%a\u002B#c\u0026caa",
"destBucketName":"minio-dotnet-example-qsokc6g6es725dj",
"data":"1KB","size":"1KB"},
"duration":73,"status":"PASS"}

We need to turn the escaping off as they are just log messages.

@martijn00
Copy link
Contributor Author

@ebozduman I've done all of those things now. Can you have a look and merge?

@martijn00
Copy link
Contributor Author

@ebozduman Can you try again with the last commit?

@martijn00
Copy link
Contributor Author

@ebozduman finally a green check! Lets merge!

Copy link
Collaborator

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

LGTM

@martijn00
Copy link
Contributor Author

@ebozduman Can you merge? I will follow up with some more warning cleanup afterwards.

@ebozduman
Copy link
Collaborator

@martijn00

Could you please rebase?

@martijn00
Copy link
Contributor Author

@ebozduman done the rebase now.

@ebozduman
Copy link
Collaborator

Hi @martijn00

looks like there are a little bit more to address. Thank you...

(cherry picked from commit 3363b25)
(cherry picked from commit 869fc5f)
@martijn00
Copy link
Contributor Author

@ebozduman fixed the merge conflicts now. Can you merge the PR?

@ebozduman ebozduman merged commit c43ef12 into minio:master Apr 13, 2023
4 checks passed
@martijn00
Copy link
Contributor Author

@ebozduman thanks for merging! When do you expect to get a new nuget release published?

@ebozduman
Copy link
Collaborator

We'll try to do it this week.

Urantij pushed a commit to Urantij/minio-dotnet that referenced this pull request Apr 19, 2023
* Start using hashing and span

* Skip some tests for now

* Working version

* Cleanup

* More span

* Cleanup

* Cleanup response

* More Span

* Fix hash

* More span

* Add netstandard compat back in

* Fix null checks

* Format

* Fix all span for netstandard

* Update package

* Add net48 compat in tests

* Fix warnings

* Fix test

* Completes changes to support 4.7.2 & 4.8 releases

* Changes to support 4.x releases

* Fix tags

* Fix file name

* turn the escaping off

* Format

* Update format

* Fix tests

* Update nuget

* Fix warnings

* More span

* Format

* Fix test

* More span

* Optimize streams

* Format again

* More memory

* Format

* Performance

* Fix test

* Progress

* More stream

* Format

* Fix response

* More span

* Use memory

* fixes the lint errors

* Fix merge

* Fixes

(cherry picked from commit 3363b25)

* More fixes

(cherry picked from commit 869fc5f)

* more lint changes

---------

Co-authored-by: Ersan <ersan.bozduman@gmail.com>
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 this pull request may close these issues.

None yet

2 participants