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

Sftp file stream should consider negative offsets in Seek #1020

Closed
wants to merge 2 commits into from

Conversation

S-Jindal
Copy link

#1018

The sftp file stream must consider negative offsets in Seek method when the caller wants to read from the end of the file using SeekOrigin.End

@S-Jindal S-Jindal requested a review from drieseng as a code owner October 25, 2022 05:42
@@ -6,6 +6,7 @@
#if FEATURE_TAP
using System.Threading.Tasks;
#endif
using System.Reflection;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this for?

Copy link
Author

Choose a reason for hiding this comment

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

Dummy change, will remove it

@zybexXL
Copy link
Contributor

zybexXL commented Oct 25, 2022

PR #910 already fixes this same issue and also refactors the code in a better way. This PR is not needed.

break;
}

if (newPosn == -1)
if (newPosn == -1 || (attributes != null && newPosn > attributes.Size))
Copy link
Contributor

Choose a reason for hiding this comment

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

seeking past the end of file when writing is valid, it's a way to extend the file size.

@S-Jindal
Copy link
Author

PR #910 already fixes this same issue and also refactors the code in a better way. This PR is not needed.

@zybexXL why isn't that PR merged yet? It was opened 8 months ago. I will abandon this in case author is willing to merge that

@zybexXL
Copy link
Contributor

zybexXL commented Oct 25, 2022

@zybexXL why isn't that PR merged yet? It was opened 8 months ago. I will abandon this in case author is willing to merge that

Good question, ask the maintainer @drieseng. There are dozens of pending PRs to merge, some for much longer than 910. This PR would have the same fate. I just merge whatever I need and build my own version instead of using the published NuGet packages.

Would someone please consider forking this to a more active project? It's clear the owner no longer has the time for this and is unwilling to add other maintainers. Think of the children!

@S-Jindal
Copy link
Author

@zybexXL why isn't that PR merged yet? It was opened 8 months ago. I will abandon this in case author is willing to merge that

Good question, ask the maintainer @drieseng. There are dozens of pending PRs to merge, some for much longer than 910. This PR would have the same fate. I just merge whatever I need and build my own version instead of using the published NuGet packages.

Would someone please consider forking this to a more active project? It's clear the owner no longer has the time for this and is unwilling to add other maintainers. Think of the children!

Ok I will try to email Gert. If he responds I will ask him to add at least one active maintainer with "owner permissions". But if he doesn't respond, would you be willing to fork it and maintain actively? And are you familiar with the release process on nuget. In .NET it isn't possible to pull develop version of the project, we need to release on nuget

@zybexXL
Copy link
Contributor

zybexXL commented Oct 25, 2022

I actually started a fork a few months back, but then decided not to go ahead for fear of not having time to maintain it (as Gert doesn't) as I'm quite busy too nowadays. I would however have no problems adding several other maintainers to the project. I may reconsider, if this project stays in zombie mode.

@S-Jindal
Copy link
Author

S-Jindal commented Nov 2, 2022

Gert has responded over email that is ready to add other maintainers to this project. @zybexXL please share your email to be part of that conversation.
Let's pull @IgorMilavec as well. I can he has contributed to several areas in this project

@zybexXL
Copy link
Contributor

zybexXL commented Nov 2, 2022

Thanks @S-Jindal, I've sent an email to you and Gert.

@IgorMilavec
Copy link
Collaborator

@S-Jindal sure, I can help out. But unless we set project governance in a drastically more open way, I'm afraid it will just raise expectations and provide little value.

I think our goal should be to provide value to SSH.NET users, specifically by:

  1. Releasing often (every commit to main should trigger a preview NuGet package build)
  2. Eliminating the gravest bugs (non-compliance with established interfaces)
  3. Supporting current platform versions

If @drieseng agrees, we should have a call to discuss project governance.

@drieseng
Copy link
Member

Duplicate of #910.

@drieseng drieseng closed this Nov 29, 2022
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.

4 participants