Introduce TempFileService and lifecycle cleanup participant for Maven 4. #11389
Conversation
| private static final Logger LOGGER = LoggerFactory.getLogger(DefaultTempFileService.class); | ||
|
|
||
| // System property to keep temp material for diagnostics. | ||
| private static final String KEEP_PROP = "maven.tempfile.keep"; |
There was a problem hiding this comment.
This should go (along with added doco) to Constants maybe?
|
@gnodet @desruisseaux I like the idea! |
|
It would be for temporary files having a longer lifetime than what we can manage with |
|
On a related note: in resolver I found this pattern quite useful (to perform cleanup immediately when tmp file was not needed anymore, but in Maven that can happen at session end I guess?): Basically, the collocated temp file is mostly used, as it creates a temp file "next to target", and after you wrote to it, on close it will be atomically moved to it's place. As this is ideal pattern to "pub-unpub" files on modern FS, like "move to place" and "remove from place", and there are no partial reads possible. |
| () -> Collections.newSetFromMap(new ConcurrentHashMap<Path, Boolean>()); | ||
|
|
||
| @Inject | ||
| public DefaultTempFileService() { |
There was a problem hiding this comment.
This empty constructor is not needed.
| } | ||
|
|
||
| @Override | ||
| public Path createTempFile(final Session session, final String prefix, final String suffix) throws IOException { |
There was a problem hiding this comment.
No need to use final on arguments or variables.
| * Service to create and track temporary files/directories for a Maven build. | ||
| * All created paths are deleted automatically when the session ends. | ||
| */ | ||
| public interface TempFileService { |
There was a problem hiding this comment.
This needs to extend Service
| * Service to create and track temporary files/directories for a Maven build. | ||
| * All created paths are deleted automatically when the session ends. | ||
| */ | ||
| public interface TempFileService { |
There was a problem hiding this comment.
Add @Nonnull for parameters and return types on all methods.
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Some adjustments, but the idea is definitely interesting.
|
Also, I think we should retarget to |
| } catch (final IOException e) { | ||
| if (first == null) { | ||
| first = e; | ||
| } |
There was a problem hiding this comment.
The other exceptions could be added as suppressed ones to the first.
Auto-clean temp files/dirs on afterSessionEnd; opt-out via -Dmaven.tempfile.keep Add tests and minimal SessionData map stub
|
I did all the requested change and also add a new pr for master. Keep this in case you want to push it to 4.x #11405 |
|
I'd rather target 4.1 |
5 participants
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.