Skip to content

Conversation

@paulijar
Copy link
Contributor

The PR #20033 added support for fseek for the S3 storage backend. However, the seek mode SEEK_END was left out that time. This PR adds this support.

The motivation for this was that the getID3 library uses the seek mode SEEK_END and doesn't work properly without it. After the PR JamesHeinrich/getID3#328 got merged, the getID3 library no longer works at all on file system where fseek returns error with any used seek mode. This was the root cause for the issue owncloud/music#887.

The PR nextcloud#20033 added support
for `fseek` for  the S3 storage backend. However, the seek mode SEEK_END
was left out that time. This PR fills this gap.

Signed-off-by: Pauli Järvinen <pauli.jarvinen@gmail.com>
@szaimen szaimen added the 3. to review Waiting for reviews label Sep 12, 2021
@szaimen szaimen added this to the Nextcloud 23 milestone Sep 12, 2021
@szaimen szaimen requested review from a team, PVince81 and blizzz and removed request for a team September 12, 2021 13:10
Copy link
Collaborator

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@blizzz blizzz requested a review from juliusknorr September 13, 2021 08:55
@juliusknorr
Copy link
Member

Failure unrelated

@juliusknorr juliusknorr merged commit 8e7f63b into nextcloud:master Sep 14, 2021
@paulijar paulijar deleted the enh/s3_seek_from_end branch September 14, 2021 12:18
@kesselb
Copy link
Collaborator

kesselb commented Sep 14, 2021

does it make sense to backport?

@paulijar
Copy link
Contributor Author

does it make sense to backport?

In my opinion, yes. This fixes an actual problem, and the patch should be applicable all the way starting from NC17.

@paulijar
Copy link
Contributor Author

So who should decide if this gets backported?

@kesselb
Copy link
Collaborator

kesselb commented Sep 20, 2021

/backport to stable22

@kesselb
Copy link
Collaborator

kesselb commented Sep 20, 2021

/backport to stable21

@kesselb
Copy link
Collaborator

kesselb commented Sep 20, 2021

/backport to stable20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants