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

Bump icewind/smb from 3.1.2 to 3.2.3 in /apps/files_external/3rdparty #37186

Conversation

dependabot-preview[bot]
Copy link
Contributor

@dependabot-preview dependabot-preview bot commented Mar 30, 2020

Bumps icewind/smb from 3.1.2 to 3.2.3.

Release notes

Sourced from icewind/smb's releases.

3.2.1

  • Reduce the number of requests needed to populate the FileInfo object when using libsmbclient-php

3.2.0

  • Throw correct exception when connection is reset
  • Allow getting acls from FileInfo
Commits
  • 5330edc remove unused method
  • 4411326 ignore any errors while trying to send the close command, the process might a...
  • b594f7c use system.dos_attr.* for both mode and stat info
  • 63319ec fix mode parsing for php 7.4
  • 9fb1753 remove debug
  • 742be89 travis fix
  • 444cc1c Revert "Use stat to fetch mode"
  • 7fc664a bumb phpunit version
  • 3c5e45d allow getting acls of files
  • acd992a Add error exception for ECONNRESET
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language
  • @dependabot badge me will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot dashboard:

  • Update frequency (including time of day and day of week)
  • Pull request limits (per update run and/or open at any time)
  • Automerge options (never/patch/minor, and dev/runtime dependencies)
  • Out-of-range updates (receive only lockfile updates, if desired)
  • Security updates (receive only security updates, if desired)

@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #37186 into master will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37186      +/-   ##
============================================
- Coverage     64.56%   64.55%   -0.02%     
  Complexity    19218    19218              
============================================
  Files          1268     1268              
  Lines         75104    75116      +12     
  Branches       1331     1331              
============================================
- Hits          48493    48492       -1     
- Misses        26219    26232      +13     
  Partials        392      392              
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.71% <0.00%> (-0.02%) 19218.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Lib/Storage/SMB.php 49.32% <0.00%> (-1.61%) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bb3236...01f8fb0. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #37186 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37186   +/-   ##
=========================================
  Coverage     64.87%   64.87%           
  Complexity    19146    19146           
=========================================
  Files          1269     1269           
  Lines         74947    74947           
  Branches       1331     1331           
=========================================
  Hits          48625    48625           
  Misses        25930    25930           
  Partials        392      392           
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 66.07% <ø> (ø) 19146.00 <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffa8467...4bd0099. Read the comment docs.

@owncloud owncloud deleted a comment from update-docs bot Mar 31, 2020
@phil-davis
Copy link
Contributor

phil-davis commented Mar 31, 2020

I pushed a changelog entry onto this PR.

@DeepDiver1975 @micbar or whoever is appropriate please review and decide if this is "a good thing"
and if there are any changes/fixes that would be reasonably urgent/useful to get into release 10.4.1 (that won't be happening - it will go in whatever upcoming release it gets in)

@mmattel
Copy link
Contributor

mmattel commented Apr 1, 2020

Reduce the number of requests needed to populate the FileInfo object when using libsmbclient-php

Which is imho a good reason...

@phil-davis phil-davis self-assigned this Apr 1, 2020
@phil-davis phil-davis requested a review from micbar April 1, 2020 11:22
@dependabot-preview
Copy link
Contributor Author

A newer version of icewind/smb exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@mmattel
Copy link
Contributor

mmattel commented Apr 14, 2020

@phil-davis can you do a miracle that v3.2.3 gets show instead of this version

@phil-davis phil-davis force-pushed the dependabot/composer/apps/files_external/3rdparty/icewind/smb-3.2.1 branch from ed83493 to c6e7ede Compare April 14, 2020 09:22
@phil-davis
Copy link
Contributor

I have rebased this PR and then added a commit to bump icewind/smb to 3.2.3 and also icewind/streams to 0.7.2

The bot just thinks that the composer.json and composer.lock need to be updated. But actually the code of these dependencies is configured in the core repo. e.g. see #31521 (comment)

Why is it like that? @PVince81 @DeepDiver1975 I guess you know the history.

So it looks like PRs such as #36017 did not really apply changes to the configured run-time code.

@phil-davis
Copy link
Contributor

I did composer update locally and committed and pushed the resulting code.
Let's see what explodes.

@phil-davis phil-davis changed the title Bump icewind/smb from 3.1.2 to 3.2.1 in /apps/files_external/3rdparty Bump icewind/smb from 3.1.2 to 3.2.3 in /apps/files_external/3rdparty Apr 14, 2020
@phil-davis
Copy link
Contributor

phil-davis commented Apr 14, 2020

https://drone.owncloud.com/owncloud/core/24253/4/7

------ --------------------------------------------------------------- 
  Line   apps/files_external/lib/Lib/Storage/SMB.php                    
 ------ --------------------------------------------------------------- 
  173    Class Icewind\SMB\Wrapped\FileInfo constructor invoked with 5  
         parameters, 6 required.                                        
  183    Class Icewind\SMB\Wrapped\FileInfo constructor invoked with 5  
         parameters, 6 required.                                        
 ------ --------------------------------------------------------------- 

 [ERROR] Found 2 errors                                                         

make: *** [test-php-phpstan] Error 1

Commit "Create new FileInfo with aclCallback that returns an empty array" should fix that.
The constructor of FileInfo requires this extra parameter. That happened in icewind1991/SMB@3c5e45d
For now, just give it a function that returns an empty array.

@phil-davis phil-davis force-pushed the dependabot/composer/apps/files_external/3rdparty/icewind/smb-3.2.1 branch from a67512c to ec12548 Compare April 14, 2020 10:45
@phil-davis
Copy link
Contributor

@DeepDiver1975 @micbar or whoever is appropriate, please review.

I guess codecov is failing because we configure a bunch of code from the dependencies and do not have our own unit tests that cover it.

@phil-davis phil-davis requested a review from jvillafanez April 16, 2020 02:15
@phil-davis phil-davis mentioned this pull request Apr 16, 2020
11 tasks
@jvillafanez
Copy link
Member

I can only give a 👍 for the update and related code changes.
In any case, I think we should verify manually that the update doesn't break anything

@micbar
Copy link
Contributor

micbar commented Apr 20, 2020

@phil-davis ping me if tested, then we can merge

@phil-davis
Copy link
Contributor

I am not familiar with how this beast fits in.
We have PHP unit tests running in CI with a samba service. Is that relevant here?
What needs to be tested manually?
(mounting and using an SMB share...?)

@jvillafanez
Copy link
Member

I think a quick smoke test should be enough

@kesselb
Copy link

kesselb commented Apr 24, 2020

Another application using that library heavily just shipped 3.2.3 to their users and it might to break some SMB setups: https://github.com/nextcloud/server/issues/20622

Bumps [icewind/smb](https://github.com/icewind1991/SMB) from 3.1.2 to 3.2.1.
- [Release notes](https://github.com/icewind1991/SMB/releases)
- [Commits](icewind1991/SMB@v3.1.2...v3.2.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
@phil-davis phil-davis force-pushed the dependabot/composer/apps/files_external/3rdparty/icewind/smb-3.2.1 branch from ec12548 to 01f8fb0 Compare May 8, 2020 09:09
@phil-davis
Copy link
Contributor

@dependabot recreate

@dependabot-preview
Copy link
Contributor Author

Superseded by #37370.

@dependabot-preview dependabot-preview bot deleted the dependabot/composer/apps/files_external/3rdparty/icewind/smb-3.2.1 branch May 11, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - To Review dependencies php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants