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

WFCORE-5201 Update commons-io to 2.8.0 #4400

Conversation

boris-unckel
Copy link
Contributor

Fixing WFCORE-5201

@wildfly-ci
Copy link

Hello, boris-unckel. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment.

@wildfly-ci wildfly-ci added the deps-ok Dependencies have been checked, and there are no significant changes label Dec 1, 2020
@bstansberry
Copy link
Contributor

@asoldano This would need an ok from you as RESTEasy is the primary consumer of this.

This upgrade breaks some cleanup logic in ModuleTestCase.addModuleWithDirectoryAndInvalidLinks2, which is concerning. It seems somewhat buggy that it breaks. The dirFile2 dir has a symlink child that points to a non-longer existent dirFile1. FileUtils is unable to delete that child.

@wildfly-ci
Copy link

Core - Full Integration Build 10156 outcome was FAILURE using a merge of 2ef507e
Summary: Tests passed: 7014, ignored: 112; failed to parse xml reports (new) Build time: 04:19:52

@boris-unckel
Copy link
Contributor Author

boris-unckel commented Dec 8, 2020

@bstansberry @asoldano The ModuleTestCase.addModuleWithDirectoryAndInvalidLinks2 fail is a commons-io known and resolved bug: IO-692
Shall I change the test to @ignore for the time till commons-io 2.9 will be published?

@bstansberry
Copy link
Contributor

@boris-unckel I wouldn't want to upgrade with a known regression unless RESTEasy had a significant interest in picking up the upgrade, so the first thing to do is see what @asoldano has to say about that. The primary reason we ship commons-io is because RESTEasy uses it.

@github-actions
Copy link

github-actions bot commented Jan 8, 2021

There has been no activity on this PR for 30 days. It will be auto-closed after 90 days.

@github-actions github-actions bot added the Stale label Jan 8, 2021
@boris-unckel boris-unckel marked this pull request as draft January 8, 2021 20:16
@boris-unckel
Copy link
Contributor Author

@bstansberry @asoldano

I wouldn't want to upgrade with a known regression unless RESTEasy had a significant interest in picking up the upgrade, so the first thing to do is see what asoldano has to say about that. The primary reason we ship commons-io is because RESTEasy uses it.

I close this PR and the related ticket without a good solution. You were perfectly right to reject the 2.8.0 version. That release is really buggy. I have ported the failing test case of WFCore to commons-io but even after several fixes it fails on Fedora machines (Ubuntu is fine). commons-io does no tests against different Linux filesystems, Ubuntu only.
If you want to check yourself simply clone and do clean install of commons-io. I have serious doubts this will be solved. Have a look at the repo to see it for yourself. Strange dev culture.
If ModuleTestCase.addModuleWithDirectoryAndInvalidLinks2 (and similar) fail in the future: The reason seems to be filesystem attributes related. You can show them with "lsattr". JUnit points to /tmp which has different attributes than i.e. /home. It's related to NIO, the delete is not proper working.
--------------e----- /home vs Unpassender IOCTL (I/O-Control) für das Gerät Beim Lesen der Flags von /tmp (German for Invalid argument While reading flags on)

@boris-unckel boris-unckel deleted the WFCORE-5201_update_commons_io branch January 30, 2021 20:34
@bstansberry
Copy link
Contributor

Thanks for the research on this @boris-unckel!

This highlights for me something I don't like about how WF dependencies work. This library is a test-only dependency in WildFly Core, but full WF pulls in the WF Core root pom in import scope. That's good for production libs, where we want a consistent version between core and full, so let core control. But it adds some fragility in that it's easy not to consider how a lib is used in full when making changes in core. (Not saying that happened here, but it easily could have.) For test-only dependencies in core I don't think that fragility is worth it.

I filed https://issues.redhat.com/browse/WFLY-14431 related to this and am thinking a bit about how to get these test-only dependencies out of the main WF Core dependencyManagement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants