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

fixed detection of server version in cmd client similar to gui client: #6016

Closed
wants to merge 3 commits into from
Closed

Conversation

errror
Copy link
Contributor

@errror errror commented Sep 1, 2023

first check status.php for version and then capabilities, only use version string if not empty

this also fixes 'File names containing the character ":" are not supported on this file system.' errors in nextcloudcmd (on Linux): The invalidFilenameRegex was set to a static default in case the server version was not set correctly. As newer versions of nextcloud do not return the version in capabilities but status.php, the server version was empty.

Signed-off-by: Patrick Cernko errror@errror.org

Patrick Cernko and others added 2 commits September 1, 2023 11:30
first check status.php for version and then capabilities, only use
version string if not empty

this also fixes 'File names containing the character ":" are not
supported on this file system.' errors in nextcloudcmd (on Linux):
The invalidFilenameRegex was set to a static default in case the server
version was not set correctly. As newer versions of nextcloud do not
return the version in capabilities but status.php, the server version
was empty.
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-6016-f8650933ee8ff1700024a0eb0a4924c2efe6aef6-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@claucambra
Copy link
Collaborator

Hi, as soon as you remove the merge commit f865093 we can merge

@claucambra
Copy link
Collaborator

Also, please make sure your relevant commit is signed off :)

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-6016-b177af9d4f6094093177d9b607ccfff127a4945d-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@errror
Copy link
Contributor Author

errror commented Sep 4, 2023

@claucambra I have no idea, what you are talking about when you say "remove the merge commit f865093". Could you please tell me, what I have to do and where?

Also how do I change my already done commit message? When I did the commit, I was not aware of the "signed off" requirement. Should I do a new fork and make the commit with the required "Signed-Off" signature?

@claucambra
Copy link
Collaborator

@claucambra I have no idea, what you are talking about when you say "remove the merge commit f865093". Could you please tell me, what I have to do and where?

You should rebase the commit on master and drop the merge commits (there are now two, f865093 and b177af9)

Also how do I change my already done commit message? When I did the commit, I was not aware of the "signed off" requirement. Should I do a new fork and make the commit with the required "Signed-Off" signature?

You should amend the commit with the --sign-off flag

@claucambra claucambra self-requested a review September 4, 2023 10:52
Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

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

Once merge commits are removed, we can merge

@errror
Copy link
Contributor Author

errror commented Sep 5, 2023

@claucambra I have no idea, what you are talking about when you say "remove the merge commit f865093". Could you please tell me, what I have to do and where?

You should rebase the commit on master and drop the merge commits (there are now two, f865093 and b177af9)

Also how do I change my already done commit message? When I did the commit, I was not aware of the "signed off" requirement. Should I do a new fork and make the commit with the required "Signed-Off" signature?

You should amend the commit with the --sign-off flag

@errror errror closed this Sep 5, 2023
@errror
Copy link
Contributor Author

errror commented Sep 5, 2023

I have created a new PR to fullfil your requirements: 6023

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.

3 participants