Skip to content

[JENKINS-47324] FilePath API in Jenkins should propagate errors #22356

Closed

Description

@jenkins-infra-bot

Almost all methods in https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/FilePath.java use obsolete pre-Java7 API, which does not propagate errors.

  • The code should be updated to java.nio.Files: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html
  • Methods should propagate errors via IOExceptions where possible (and log errors to java.util.logging.Logger otherwise)
  • Runtime exceptions from the new API should be caught if the methods throw them (e.g. InvalidPathException)

Originally reported by oleg_nenashev, imported from: FilePath API in Jenkins should propagate errors
  • assignee: krishbhasin
  • status: Closed
  • priority: Minor
  • component(s): core
  • label(s): diagnostics, missing-acceptance-criteria, needs-details, newbie-friendly
  • resolution: Done
  • resolved: 2019-01-11T16:21:59+00:00
  • votes: 1
  • watchers: 7
  • imported: 2025-11-24
Raw content of original issue

Almost all methods in https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/FilePath.java use obsolete pre-Java7 API, which does not propagate errors.

  • The code should be updated to java.nio.Files: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html
  • Methods should propagate errors via IOExceptions where possible (and log errors to java.util.logging.Logger otherwise)
  • Runtime exceptions from the new API should be caught if the methods throw them (e.g. InvalidPathException)

Activity

  1. jenkins-infra-bot commented on Oct 6, 2017

    @jenkins-infra-bot
    Author
  2. jenkins-infra-bot commented on Oct 16, 2017

    @jenkins-infra-bot
    Author
    krishbhasin:
    • Original comment link
    • Raw content of original comment:

      Is this issue taken? I'd like to have a crack at it if not

    Is this issue taken? I'd like to have a crack at it if not

  3. jenkins-infra-bot commented on Oct 16, 2017

    @jenkins-infra-bot
    Author
    oleg_nenashev:
    • Original comment link
    • Raw content of original comment:

      krishbhasin yes. marykomar has taken the issue, but so far there is no progress on it AFAICT. There may be other issues around of such kind. Actually you can look for any obsolete Java 6 File API usage (File#mkdirs() & Co) in the core and create patches for that

    krishbhasin yes. marykomar has taken the issue, but so far there is no progress on it AFAICT. There may be other issues around of such kind. Actually you can look for any obsolete Java 6 File API usage (File#mkdirs() & Co) in the core and create patches for that

  4. jenkins-infra-bot commented on Oct 17, 2017

    @jenkins-infra-bot
    Author
    marykomar:
    • Original comment link
    • Raw content of original comment:

      Yes, I'm working on it.

    Yes, I'm working on it.

  5. jenkins-infra-bot commented on Oct 18, 2017

    @jenkins-infra-bot
    Author
    marykomar:
    • Original comment link
    • Raw content of original comment:

      krishbhasin my plans have changed, and I, unfortunately, can't work on this issue, so if you want you can take it.

    krishbhasin my plans have changed, and I, unfortunately, can't work on this issue, so if you want you can take it.

  6. jenkins-infra-bot commented on Oct 19, 2017

    @jenkins-infra-bot
    Author
    batmat:
    • Original comment link
    • Raw content of original comment:

      I've assigned krishbhasin to clarify. Thanks a lot to you both to try and help! If you need any help, please do not hesitate to come ask questions and discuss on the jenkins-developers mailing list!

    I've assigned krishbhasin to clarify. Thanks a lot to you both to try and help! If you need any help, please do not hesitate to come ask questions and discuss on the jenkins-developers mailing list!

  7. jenkins-infra-bot commented on Nov 6, 2017

    @jenkins-infra-bot
    Author
    krishbhasin:
    • Original comment link
    • Raw content of original comment:

      I've had some fun getting the Messages wired into my IDE, it still complains that it doesn't know what to do with the fields...

       

      With the actual code changes, I'm not sure how far 'up the chain' it needs to go. There seem to be utility methods (like filterNonNull) that are used a bit, that return a 'SoloFilePathFilter', defined further up in the project. Should I also start modifying that class to make this work?

    I've had some fun getting the Messages wired into my IDE, it still complains that it doesn't know what to do with the fields...

     

    With the actual code changes, I'm not sure how far 'up the chain' it needs to go. There seem to be utility methods (like filterNonNull) that are used a bit, that return a 'SoloFilePathFilter', defined further up in the project. Should I also start modifying that class to make this work?

  8. jenkins-infra-bot commented on Nov 6, 2017

    @jenkins-infra-bot
    Author
    krishbhasin:
    • Original comment link
    • Raw content of original comment:

      I should say, I'm using this to help with the code changes.

    I should say, I'm using this to help with the code changes.

  9. jenkins-infra-bot commented on Nov 10, 2017

    @jenkins-infra-bot
    Author
    dnusbaum:
    • Original comment link
    • Raw content of original comment:

      krishbhasin IMHO I probably wouldn't go that far just yet. I'd focus on replacing the calls to methods like File#renameTo and File#mkdirs with calls to Files.move and Files.createDirectories. I believe many of the methods that take File as argument can't be converted to take Path because they are being sent over remoting channels and Path isn't serializable. File#toPath is your friend!

      I was working on some things related to this before I realized there was an existing ticket being worked on by someone else, so feel free to look at the changes in my branch in case they are useful to you. I'll stop working on that branch and leave the ticket in your hands for now

    krishbhasin IMHO I probably wouldn't go that far just yet. I'd focus on replacing the calls to methods like File#renameTo and File#mkdirs with calls to Files.move and Files.createDirectories. I believe many of the methods that take File as argument can't be converted to take Path because they are being sent over remoting channels and Path isn't serializable. File#toPath is your friend!

    I was working on some things related to this before I realized there was an existing ticket being worked on by someone else, so feel free to look at the changes in my branch in case they are useful to you. I'll stop working on that branch and leave the ticket in your hands for now

  10. jenkins-infra-bot commented on Nov 11, 2017

    @jenkins-infra-bot
    Author
    krishbhasin:
    • Original comment link
    • Raw content of original comment:

      I've had a look at your changes, and they make sense, but I feel a bit out of my depth trying to migrate any further methods over to the Java 7 API... I'll give it a go and put what I have in a PR, and maybe someone can help give me pointers from there?

    I've had a look at your changes, and they make sense, but I feel a bit out of my depth trying to migrate any further methods over to the Java 7 API... I'll give it a go and put what I have in a PR, and maybe someone can help give me pointers from there?

  11. jenkins-infra-bot commented on Nov 14, 2017

    @jenkins-infra-bot
    Author
    dnusbaum:
    • Original comment link
    • Raw content of original comment:

      krishbhasin That sounds like a good plan to me, any improvement will be welcome, and there's no need to do everything at once. Feel free to mention @dwnusbaum on the PR and I'll take a look at it.

    krishbhasin That sounds like a good plan to me, any improvement will be welcome, and there's no need to do everything at once. Feel free to mention @​dwnusbaum on the PR and I'll take a look at it.

  12. jenkins-infra-bot commented on Nov 29, 2017

    @jenkins-infra-bot
    Author
    dnusbaum:

    krishbhasin Do you still plan on filing a PR for this?

  13. jenkins-infra-bot commented on Nov 29, 2017

    @jenkins-infra-bot
    Author
    krishbhasin:
    • Original comment link
    • Raw content of original comment:

      dnusbaum Yeah sorry it's been a busy few weeks (holiday then knocked out with flu). I will get to this before the end of the year, I'm still recovering from the illness.

    dnusbaum Yeah sorry it's been a busy few weeks (holiday then knocked out with flu). I will get to this before the end of the year, I'm still recovering from the illness.

  14. jenkins-infra-bot commented on Nov 29, 2017

    @jenkins-infra-bot
    Author
    dnusbaum:
    • Original comment link
    • Raw content of original comment:

      krishbhasin Ok that sounds good. I hope you're feeling better soon.

    krishbhasin Ok that sounds good. I hope you're feeling better soon.

  15. jenkins-infra-bot commented on Dec 14, 2017

    @jenkins-infra-bot
    Author
    scm_issue_link:
    • Original comment link
    • Raw content of original comment:

      Code changed in jenkins User: Krishan Bhasin Path: core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/util/IOUtils.java http://jenkins-ci.org/commit/jenkins/09bcc5d6538b3cfffbf71228ebd1679e3e20d8b2 Log: JENKINS-47324 - Reduce usage of File.mkdirs() in FilePath and IOUtils (#3173)

      • Move an instance of renameTo() to Files.move()
      • Replace an instance of File.toURI() with an instance of Path.toUri()
      • Replace mkdirs() with Files.createDirectories() Replace mkdir() with Files.createTempDirectory()
      • Undo addition of createTempDirectory() as per review comments
      • Return to use of FilePath#mkdirs(File) and instead modify it to use the new API.
      • Undo addition of toPath() in a URI conversion as it brings no benefits.
      • Replace new uses of toPath() with Util.fileToPath() to pre-handle runtime exceptions
      • Remove * import. move mkdirs() to using FilePath method instead of File method.
      • Make IOUtils.mkdirs(File) use Java7 API calls
      • Add back accidentally removed imports.
      • Fixed use of wildcard import
      • Use utility method fileToPath() to handle potential exception thrown

    Code changed in jenkins
    User: Krishan Bhasin
    Path:
    core/src/main/java/hudson/FilePath.java
    core/src/main/java/hudson/util/IOUtils.java
    http://jenkins-ci.org/commit/jenkins/09bcc5d6538b3cfffbf71228ebd1679e3e20d8b2
    Log:
    JENKINS-47324 - Reduce usage of File.mkdirs() in FilePath and IOUtils (#3173)

    • Move an instance of renameTo() to Files.move()
    • Replace an instance of File.toURI() with an instance of Path.toUri()
    • Replace mkdirs() with Files.createDirectories()
      Replace mkdir() with Files.createTempDirectory()
    • Undo addition of createTempDirectory() as per review comments
    • Return to use of FilePath#mkdirs(File) and instead modify it to use the new API.
    • Undo addition of toPath() in a URI conversion as it brings no benefits.
    • Replace new uses of toPath() with Util.fileToPath() to pre-handle runtime exceptions
    • Remove * import.
      move mkdirs() to using FilePath method instead of File method.
    • Make IOUtils.mkdirs(File) use Java7 API calls
    • Add back accidentally removed imports.
    • Fixed use of wildcard import
    • Use utility method fileToPath() to handle potential exception thrown
  16. jenkins-infra-bot commented on Dec 17, 2017

    @jenkins-infra-bot
    Author
    danielbeck:

    oleg_nenashev Is this issue fixed towards 2.96 or still WIP?

  17. jenkins-infra-bot commented on Dec 17, 2017

    @jenkins-infra-bot
    Author
    oleg_nenashev:
    • Original comment link
    • Raw content of original comment:

      danielbeck IIUC there are still some changes to be done in this ticket

    danielbeck IIUC there are still some changes to be done in this ticket

  18. jenkins-infra-bot commented on Jan 10, 2019

    @jenkins-infra-bot
    Author
    batmat:

    I feel like the acceptance criteria are slightly unclear, then it makes it hard to be sure this is done or not. I see at least the PR from krishbhasin and dnusbaum in #3135

     oleg_nenashev given you filed this, *when you have time* could you possibly add acceptance criteria so it's easily assessable by anyone if/when this would be done?

    Spassiba!

  19. jenkins-infra-bot commented on Jan 10, 2019

    @jenkins-infra-bot
    Author
    batmat:
    • Original comment link
    • Raw content of original comment:

      krishbhasin would you be BTW be available/willing to handle the possible remainder of this task, if any? We're happy to provide any guidance for this. Thanks!

    krishbhasin would you be BTW be available/willing to handle the possible remainder of this task, if any? We're happy to provide any guidance for this. Thanks!

  20. jenkins-infra-bot commented on Jan 10, 2019

    @jenkins-infra-bot
    Author
    dnusbaum:

    Krishan submitted #3173, #3135 from me was more related to JENKINS-36088. There are likely other things that could be addressed, but I think we got most of the lowest-hanging fruit looking at the related issues and glancing quickly through FilePath.java.

  21. jenkins-infra-bot commented on Jan 11, 2019

    @jenkins-infra-bot
    Author
    batmat:
    • Original comment link
    • Raw content of original comment:

      OK, then let's consider this fixed until proven otherwise. Like I wrote above anyway, it can be reopened anytime once we know about left things to be still fixed. Thanks for the feedback Devin.

    OK, then let's consider this fixed until proven otherwise. Like I wrote above anyway, it can be reopened anytime once we know about left things to be still fixed. Thanks for the feedback Devin.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions