6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance #2618

bplb · 2021-02-17T21:48:16Z

Please review this minor specification update to highlight that File.renameTo(File) does not modify the File instance on which the method is invoked.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance

Reviewers

Roger Riggs (@RogerRiggs - Reviewer)
Naoto Sato (@naotoj - Reviewer) ⚠️ Review applies to e7ff82d
Alan Bateman (@AlanBateman - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2618/head:pull/2618
$ git checkout pull/2618

…ot the File instance

bridgekeeper · 2021-02-17T21:49:23Z

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-02-17T21:50:23Z

@bplb The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

bplb · 2021-02-17T21:52:29Z

/csr

openjdk · 2021-02-17T21:52:40Z

@bplb has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@bplb please create a CSR request and add link to it in JDK-6245663. This pull request cannot be integrated until the CSR request is approved.

mlbridge · 2021-02-17T21:55:52Z

Webrevs

RogerRiggs · 2021-02-17T22:04:38Z

src/java.base/share/classes/java/io/File.java

+     * that the rename operation was successful.  As instances of {@code File}
+     * are immutable, the abstract pathname represented by this {@code File}
+     * object does not itself change although the filesystem object it denoted
+     * might have moved.
     *


Is there a way to mix in a mention of the {@ code "dest"} argument.
"might have moved to the abstract pathname provided in the dest argument".

naotoj · 2021-02-17T22:24:53Z

src/java.base/share/classes/java/io/File.java

-     * that the rename operation was successful.
+     * that the rename operation was successful.  As instances of {@code File}
+     * are immutable, the abstract pathname represented by this {@code File}
+     * object does not itself change although the filesystem object it denoted


I guess you meant file object here, instead of filesystem?

No, I intended filesystem but in the sense of "object in the filesystem" but it does seem awkward.

It might be clearer if the end of the sentence were changed to something like "... this File object is not changed to name destination file or directory".

Like so?

--- a/src/java.base/share/classes/java/io/File.java +++ b/src/java.base/share/classes/java/io/File.java @@ -1376,7 +1376,9 @@ public class File * file from one filesystem to another, it might not be atomic, and it * might not succeed if a file with the destination abstract pathname * already exists. The return value should always be checked to make sure - * that the rename operation was successful. + * that the rename operation was successful. As instances of {@code File} + * are immutable, this File object is not changed to name the destination + * file or directory. * * <p> Note that the {@link java.nio.file.Files} class defines the {@link * java.nio.file.Files#move move} method to move or rename a file in a

yes, I think that works.

CSR also so updated.

naotoj

Thanks. Looks good.

openjdk · 2021-02-19T02:58:00Z

@bplb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance

Reviewed-by: rriggs, naoto, alanb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 17 new commits pushed to the master branch:

7e78c77: 8261905: Move implementation of OopStorage num_dead related functions
78cde64: 8261860: Crash caused by lambda proxy class loaded in Shutdown hook
c158413: 8261098: Add clhsdb "findsym" command
0c31d5b: 8261977: Fix comment for getPrefixed() in canonicalize_md.c
9cf4f90: 8261473: Shenandoah: Add breakpoint support
c4664e6: 8261940: Fix references to IOException in BigDecimal javadoc
0e9c5ae: 8075909: [TEST_BUG] The regression-swing case failed as it does not have the 'Open' button when select 'subdir' folder with NimbusLAF
e9f3aab: 8261912: Code IfNode::fold_compares_helper more defensively
fd098e7: 8261838: Shenandoah: reconsider heap region iterators memory ordering
f94a845: 8261600: NMT: Relax memory order for updating MemoryCounter and fix racy updating of peak values
... and 7 more: https://git.openjdk.java.net/jdk/compare/d5a4d2266b45107cf8d7c0d0137c77b797d836b6...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

bplb · 2021-02-19T16:35:12Z

/integrate

openjdk · 2021-02-19T16:36:16Z

@bplb Since your change was applied there have been 28 commits pushed to the master branch:

c4f17a3: 8257925: enable more support for nested inline tags
433096a: 8261744: Implement CharsetDecoder ASCII and latin-1 fast-paths
efbaede: 8262018: Wrong format in SAP copyright header of OsVersionTest
55463b0: 8261984: Shenandoah: Remove unused ShenandoahPushWorkerQueuesScope class
a180a38: 8260694: (fc) Clarify FileChannel.transferFrom to better describe "no bytes available" case
1b0c36b: 8261649: AArch64: Optimize LSE atomics in C++ code
61820b7: 8259984: IGV: Crash when drawing control flow before GCM
7e2c909: 8260485: Simplify and unify handler vectors in Posix signal code
c99eeb0: 8260858: Implementation specific property xsltcIsStandalone for XSLTC Serializer
5caf686: 8261644: NMT: Simplifications and cleanups
... and 18 more: https://git.openjdk.java.net/jdk/compare/d5a4d2266b45107cf8d7c0d0137c77b797d836b6...master

Your commit was automatically rebased without conflicts.

Pushed as commit 851b2e3.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

6245663: (spec) File.renameTo(File) changes the file-system object, n…

a8e2cb1

…ot the File instance

openjdk bot added the rfr label Feb 17, 2021

openjdk bot added the core-libs label Feb 17, 2021

openjdk bot added the csr label Feb 17, 2021

RogerRiggs reviewed Feb 17, 2021

View changes

6245663: Mention 'dest' parameter in the added doc

43e46a3

naotoj reviewed Feb 17, 2021

View changes

RogerRiggs approved these changes Feb 17, 2021

View changes

6245663: Update copyright year.

e7ff82d

naotoj approved these changes Feb 17, 2021

View changes

6245663: Simplify new verbiage

e6a23ab

AlanBateman approved these changes Feb 18, 2021

View changes

RogerRiggs approved these changes Feb 18, 2021

View changes

openjdk bot removed the csr label Feb 19, 2021

openjdk bot added the ready label Feb 19, 2021

openjdk bot closed this Feb 19, 2021

openjdk bot added integrated and removed ready rfr labels Feb 19, 2021

bplb deleted the 6245663-File-renameTo branch Feb 19, 2021

6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance #2618

6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance #2618

bplb commented Feb 17, 2021 •

edited by openjdk[bot] bot

bridgekeeper bot commented Feb 17, 2021

openjdk bot commented Feb 17, 2021

bplb commented Feb 17, 2021

openjdk bot commented Feb 17, 2021

mlbridge bot commented Feb 17, 2021 •

edited

RogerRiggs Feb 17, 2021

bplb Feb 17, 2021

naotoj Feb 17, 2021

bplb Feb 17, 2021

AlanBateman Feb 18, 2021

bplb Feb 18, 2021

AlanBateman Feb 18, 2021

bplb Feb 18, 2021

naotoj left a comment

openjdk bot commented Feb 19, 2021 •

edited

bplb commented Feb 19, 2021

openjdk bot commented Feb 19, 2021

6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance #2618

6245663: (spec) File.renameTo(File) changes the file-system object, not the File instance #2618

Conversation

bplb commented Feb 17, 2021 • edited by openjdk[bot] bot

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Feb 17, 2021

openjdk bot commented Feb 17, 2021

bplb commented Feb 17, 2021

openjdk bot commented Feb 17, 2021

mlbridge bot commented Feb 17, 2021 • edited

Webrevs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naotoj left a comment

openjdk bot commented Feb 19, 2021 • edited

bplb commented Feb 19, 2021

openjdk bot commented Feb 19, 2021

bplb commented Feb 17, 2021 •

edited by openjdk[bot] bot

mlbridge bot commented Feb 17, 2021 •

edited

openjdk bot commented Feb 19, 2021 •

edited