From 5a99c43efb817986510c0ade47fd12b1cc1883f2 Mon Sep 17 00:00:00 2001 From: Jesper Gravgaard Date: Mon, 2 Nov 2020 15:00:09 +0100 Subject: [PATCH] Fixed problem where temporary folders not being deleted causes errors on every compile. Closes #493 --- .gitignore | 2 + src/main/java/dk/camelot64/kickc/KickC.java | 6 + .../dk/camelot64/kickc/TmpDirManager.java | 137 ++++++++++++++++++ .../kickc/passes/Pass5FixLongBranches.java | 48 ++---- .../camelot64/kickc/test/TestKickAssRun.java | 21 ++- .../dk/camelot64/kickc/test/TestPrograms.java | 5 + 6 files changed, 174 insertions(+), 45 deletions(-) create mode 100644 src/main/java/dk/camelot64/kickc/TmpDirManager.java diff --git a/.gitignore b/.gitignore index 35411a4a6..582aefa19 100644 --- a/.gitignore +++ b/.gitignore @@ -3,9 +3,11 @@ */*.brk */*.prg */*.sym +*/.tmpdirs */bin/ */workspace.xml ./target/ target/ **/.DS_Store .project +.tmpdirs diff --git a/src/main/java/dk/camelot64/kickc/KickC.java b/src/main/java/dk/camelot64/kickc/KickC.java index f3f739d7e..a139e92ca 100644 --- a/src/main/java/dk/camelot64/kickc/KickC.java +++ b/src/main/java/dk/camelot64/kickc/KickC.java @@ -220,6 +220,9 @@ public class KickC implements Callable { Program program = compiler.getProgram(); + // Initialize tmp dir manager + TmpDirManager.init(program.getAsmFragmentBaseFolder()); + // Initialize the master ASM fragment synthesizer program.initAsmFragmentMasterSynthesizer(!optimizeNoFragmentCache); @@ -486,6 +489,9 @@ public class KickC implements Callable { } } + if(TmpDirManager.MANAGER!=null) + TmpDirManager.MANAGER.cleanup(); + return CommandLine.ExitCode.OK; } diff --git a/src/main/java/dk/camelot64/kickc/TmpDirManager.java b/src/main/java/dk/camelot64/kickc/TmpDirManager.java new file mode 100644 index 000000000..2c436c41e --- /dev/null +++ b/src/main/java/dk/camelot64/kickc/TmpDirManager.java @@ -0,0 +1,137 @@ +package dk.camelot64.kickc; + +import dk.camelot64.kickc.model.CompileError; + +import java.io.*; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +/** + * Manages temporary folders with files. + * KickAssembler holds open handles to compiled ASM files. Therefore they cannot be deleted immediately after compilation. + * This manager + * - supports creation of new temporary folders + * - attempts deletion on exit + * - if deletion is not successful the folder paths are saved to a file. The folders in this file is attempted deleted on the next invocation. + */ +public class TmpDirManager { + + /** Singleton manager. */ + public static TmpDirManager MANAGER; + + /** + * Initialize the singleton manager + * + * @param baseFolder The base folder where the TXT file containing dirs to delete will be loaded/saved from. + */ + public static void init(Path baseFolder) { + MANAGER = new TmpDirManager(baseFolder); + } + + /** The base folder where the TXT file containing dirs to delete will be loaded/saved from. */ + private Path baseFolder; + + /** Tmp folders that have been created and not deleted. */ + private List tmpDirs; + + private TmpDirManager(Path baseFolder) { + this.baseFolder = baseFolder; + this.tmpDirs = new ArrayList<>(); + } + + /** + * Creates a new temporary directory + * + * @return The new temporary folder + */ + public Path newTmpDir() { + try { + Path tmpDir = Files.createTempDirectory("kickc"); + this.tmpDirs.add(tmpDir); + return tmpDir; + } catch(IOException e) { + throw new CompileError("Error creating temporary directory. ", e); + } + } + + /** + * Deletes all temporary folders - including any files in the folders. + * If deletion is not successful the absolute paths are saved to a txt-file to tried again later. + * This also loads the txt-file with previous failed deletions and retries them + */ + public void cleanup() { + try { + // Attempt deletion of all temporary folders - including all files in the folders + List failedDirs = new ArrayList<>(); + for(Path tmpDir : tmpDirs) { + boolean success = deleteTmpDir(tmpDir); + if(!success) { + failedDirs.add(tmpDir); + //System.out.println("Cannot delete temporary folder, postponing " + tmpDir); + } else { + //System.out.println("Successfully deleted temporary folder " + tmpDir); + } + } + // Read postponed file and delete any paths + File todoFile = baseFolder.resolve(".tmpdirs").toFile(); + if(todoFile.exists()) { + FileReader todoFileReader = new FileReader(todoFile); + BufferedReader todoBufferedReader = new BufferedReader(todoFileReader); + String todoPathAbs = todoBufferedReader.readLine(); + while(todoPathAbs != null) { + Path todoPath = new File(todoPathAbs).toPath(); + boolean success = deleteTmpDir(todoPath); + if(!success) { + failedDirs.add(todoPath); + //System.out.println("Cannot delete postponed temporary folder - postponing again " + todoPathAbs); + } else { + //System.out.println("Successfully deleted postponed temporary folder " + todoPathAbs); + } + todoPathAbs = todoBufferedReader.readLine(); + } + todoBufferedReader.close(); + todoFileReader.close(); + } + // Delete the old postponed file + if(todoFile.exists()) { + if(!todoFile.delete()) + System.err.println("Warning! Cannot delete .tmpdir file " + todoFile.getAbsolutePath()); + //System.out.println("Deleted old .tmpdir file " + todoFile.getAbsolutePath()); + } + // Save any failed paths to new postponed file + if(failedDirs.size() > 0) { + PrintStream todoPrintStream = new PrintStream(todoFile); + for(Path failedDir : failedDirs) { + todoPrintStream.println(failedDir.toAbsolutePath()); + } + todoPrintStream.close(); + //System.out.println("Saved .tmpdir file with " + failedDirs.size() + " postponed temporary folders " + todoFile.getAbsolutePath()); + } + } catch(IOException e) { + throw new CompileError("Error cleaning up temporary files", e); + } + } + + private boolean deleteTmpDir(Path tmpDir) { + // Delete the temporary directory with folders + boolean success = true; + String[] entries = tmpDir.toFile().list(); + if(entries != null) + for(String s : entries) { + File currentFile = new File(tmpDir.toFile(), s); + if(!currentFile.delete()) { + //System.err.println("Warning! Cannot delete temporary file " + currentFile.getAbsolutePath()); + success = false; + break; + } + } + if(!tmpDir.toFile().delete()) { + //System.err.println("Warning! Cannot delete temporary folder " + tmpDir.toAbsolutePath()); + success = false; + } + return success; + } + +} diff --git a/src/main/java/dk/camelot64/kickc/passes/Pass5FixLongBranches.java b/src/main/java/dk/camelot64/kickc/passes/Pass5FixLongBranches.java index 7c8b71a7d..df06f5bfa 100644 --- a/src/main/java/dk/camelot64/kickc/passes/Pass5FixLongBranches.java +++ b/src/main/java/dk/camelot64/kickc/passes/Pass5FixLongBranches.java @@ -2,6 +2,7 @@ package dk.camelot64.kickc.passes; import dk.camelot64.cpufamily6502.CpuAddressingMode; import dk.camelot64.cpufamily6502.CpuOpcode; +import dk.camelot64.kickc.TmpDirManager; import dk.camelot64.kickc.asm.*; import dk.camelot64.kickc.model.CompileError; import dk.camelot64.kickc.model.Program; @@ -21,8 +22,6 @@ import java.util.regex.Pattern; */ public class Pass5FixLongBranches extends Pass5AsmOptimization { - private Path tmpDir; - public Pass5FixLongBranches(Program program) { super(program); } @@ -54,35 +53,25 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization { private boolean step() { // Reindex ASM lines new Pass5ReindexAsmLines(getProgram()).optimize(); - - // Create a temporary directory for the ASM file - try { - tmpDir = Files.createTempDirectory("kickc"); - } catch(IOException e) { - throw new CompileError("Error creating temp file.", e); - } - + Path tmpDir = TmpDirManager.MANAGER.newTmpDir(); // Generate the ASM file String outputFileName = getProgram().getPrimaryFileName(); try { //getLog().append("ASM"); //getLog().append(getProgram().getAsm().toString(false, true)); - - writeOutputFile(outputFileName, ".asm", getProgram().getAsm().toString(new AsmProgram.AsmPrintState(false), null)); - + writeOutputFile(tmpDir, outputFileName, ".asm", getProgram().getAsm().toString(new AsmProgram.AsmPrintState(false), null)); // Copy Resource Files for(Path asmResourceFile : getProgram().getAsmResourceFiles()) { - File binFile = getTmpFile(asmResourceFile.getFileName().toString()); + File binFile = getTmpFile(tmpDir, asmResourceFile.getFileName().toString()); Files.copy(asmResourceFile, binFile.toPath()); } - } catch(IOException e) { throw new CompileError("Error writing ASM temp file.", e); } // Compile using KickAssembler - catch the output in a String - File asmFile = getTmpFile(outputFileName, ".asm"); - File binaryFile = getTmpFile(outputFileName, "."+getProgram().getTargetPlatform().getOutFileExtension()); + File asmFile = getTmpFile(tmpDir, outputFileName, ".asm"); + File binaryFile = getTmpFile(tmpDir, outputFileName, "."+getProgram().getTargetPlatform().getOutFileExtension()); ByteArrayOutputStream kickAssOut = new ByteArrayOutputStream(); System.setOut(new PrintStream(kickAssOut)); int asmRes = -1; @@ -117,7 +106,6 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization { // Found line number //getLog().append("Found long branch line number "+contextLineIdx); if(fixLongBranch(contextLineIdx - 1)) { - removeTmpDir(); return true; } } @@ -125,25 +113,9 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization { } } } - - removeTmpDir(); return false; } - private void removeTmpDir() { - // Delete the temporary directory with folders - String[]entries = tmpDir.toFile().list(); - for(String s: entries){ - File currentFile = new File(tmpDir.toFile(),s); - if(!currentFile.delete()) { - System.err.println("Warning! Cannot delete temporary file "+currentFile.getAbsolutePath()); - } - } - if(!tmpDir.toFile().delete()) { - System.err.println("Warning! Cannot delete temporary folder "+tmpDir.toAbsolutePath()); - } - } - /** * Fix a long branch detected at a specific ASM index * @@ -202,9 +174,9 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization { } } - public File writeOutputFile(String fileName, String extension, String outputString) throws IOException { + private File writeOutputFile(Path tmpDir, String fileName, String extension, String outputString) throws IOException { // Write output file - File file = getTmpFile(fileName, extension); + File file = getTmpFile(tmpDir, fileName, extension); FileOutputStream outputStream = new FileOutputStream(file); OutputStreamWriter writer = new OutputStreamWriter(outputStream); writer.write(outputString); @@ -214,12 +186,12 @@ public class Pass5FixLongBranches extends Pass5AsmOptimization { return file; } - public File getTmpFile(String fileName, String extension) { + private static File getTmpFile(Path tmpDir, String fileName, String extension) { Path kcPath = FileSystems.getDefault().getPath(fileName); return new File(tmpDir.toFile(), kcPath.getFileName().toString() + extension); } - public File getTmpFile(String fileName) { + private static File getTmpFile(Path tmpDir, String fileName) { return new File(tmpDir.toFile(), fileName ); } diff --git a/src/test/java/dk/camelot64/kickc/test/TestKickAssRun.java b/src/test/java/dk/camelot64/kickc/test/TestKickAssRun.java index 319e9daa7..03ecfc3a0 100644 --- a/src/test/java/dk/camelot64/kickc/test/TestKickAssRun.java +++ b/src/test/java/dk/camelot64/kickc/test/TestKickAssRun.java @@ -1,5 +1,6 @@ package dk.camelot64.kickc.test; +import dk.camelot64.kickc.TmpDirManager; import kickass.KickAssembler65CE02; import kickass.nonasm.c64.CharToPetsciiConverter; import org.junit.jupiter.api.Test; @@ -19,16 +20,22 @@ public class TestKickAssRun { */ @Test public void testKickAssRun() throws IOException, URISyntaxException { + TmpDirManager.init(new File("").toPath()); + ReferenceHelper asmHelper = new ReferenceHelperFolder("src/test/java/dk/camelot64/kickc/test/"); URI asmUri = asmHelper.loadReferenceFile("kickasstest", ".asm"); Path asmPath = Paths.get(asmUri); - File asmPrgFile = getTmpFile("kickasstest", ".prg"); + + Path tmpDir = TmpDirManager.MANAGER.newTmpDir(); + File asmFile = getTmpFile(tmpDir, "kickasstest", ".asm"); + File asmPrgFile = getTmpFile(tmpDir, "kickasstest", ".prg"); + Files.copy(asmPath, asmFile.toPath()); ByteArrayOutputStream kickAssOut = new ByteArrayOutputStream(); System.setOut(new PrintStream(kickAssOut)); try { CharToPetsciiConverter.setCurrentEncoding("screencode_mixed"); - KickAssembler65CE02.main2(new String[]{asmPath.toAbsolutePath().toString(), "-o", asmPrgFile.getAbsolutePath()}); - } catch (AssertionError e) { + KickAssembler65CE02.main2(new String[]{asmFile.getAbsolutePath(), "-o", asmPrgFile.getAbsolutePath()}); + } catch(AssertionError e) { System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out))); String output = kickAssOut.toString(); System.out.println(output); @@ -38,11 +45,11 @@ public class TestKickAssRun { } String output = kickAssOut.toString(); System.out.println(output); + + TmpDirManager.MANAGER.cleanup(); } - - public File getTmpFile(String fileName, String extension) throws IOException { - Path tmpDir = Files.createTempDirectory("kickc"); + public static File getTmpFile(Path tmpDir, String fileName, String extension) throws IOException { Path kcPath = FileSystems.getDefault().getPath(fileName); return new File(tmpDir.toFile(), kcPath.getFileName().toString() + extension); } @@ -90,7 +97,7 @@ public class TestKickAssRun { private void printPetscii(String encoding, char ch, String sCh) { CharToPetsciiConverter.setCurrentEncoding(encoding); Byte petscii = CharToPetsciiConverter.convert(ch); - System.out.println(encoding+": "+sCh+" > "+(petscii==null?"null":(int)petscii)); + System.out.println(encoding + ": " + sCh + " > " + (petscii == null ? "null" : (int) petscii)); } diff --git a/src/test/java/dk/camelot64/kickc/test/TestPrograms.java b/src/test/java/dk/camelot64/kickc/test/TestPrograms.java index e05ddfbf1..67357a1e9 100644 --- a/src/test/java/dk/camelot64/kickc/test/TestPrograms.java +++ b/src/test/java/dk/camelot64/kickc/test/TestPrograms.java @@ -3,6 +3,7 @@ package dk.camelot64.kickc.test; import dk.camelot64.kickc.CompileLog; import dk.camelot64.kickc.Compiler; import dk.camelot64.kickc.SourceLoader; +import dk.camelot64.kickc.TmpDirManager; import dk.camelot64.kickc.asm.AsmProgram; import dk.camelot64.kickc.model.CompileError; import dk.camelot64.kickc.model.Program; @@ -68,6 +69,7 @@ public class TestPrograms { public void testAtariXlMd5b() throws IOException, URISyntaxException { compileAndCompare("atarixl-md5b.c"); } + @Test public void testAtariXlMd5() throws IOException, URISyntaxException { compileAndCompare("atarixl-md5.c"); @@ -4822,10 +4824,13 @@ public class TestPrograms { @BeforeAll public static void setUp() { + TmpDirManager.init(new File("").toPath()); } @AfterAll public static void tearDown() { + if(TmpDirManager.MANAGER != null) + TmpDirManager.MANAGER.cleanup(); //AsmFragmentTemplateUsages.logUsages(log, false, false, false, false, false, false); //printGCStats(); }