Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

The files target property: Add support for directories #1053

Merged
merged 6 commits into from
Mar 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions org.lflang/src/org/lflang/generator/c/CGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ public void accommodatePhysicalActionsIfPresent() {
var actions = Iterables.filter(IteratorExtensions.toIterable(resource.getAllContents()), Action.class);
for (Action action : actions) {
if (Objects.equal(action.getOrigin(), ActionOrigin.PHYSICAL)) {
// If the unthreaded runtime is requested, use the threaded runtime instead
// If the unthreaded runtime is not requested by the user, use the threaded runtime instead
// because it is the only one currently capable of handling asynchronous events.
if (!targetConfig.threading) {
if (!targetConfig.threading && !targetConfig.setByUser.contains(TargetProperty.THREADING)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about this change. What does it do? I seems entirely unrelated to the PR.

targetConfig.threading = true;
errorReporter.reportWarning(
action,
Expand Down Expand Up @@ -1028,7 +1028,7 @@ private void inspectReactorEResource(ReactorDecl reactor) {
}

/**
* Copy all files listed in the target property `files` and `cmake-include`
* Copy all files or directories listed in the target property `files` and `cmake-include`
* into the src-gen folder of the main .lf file
*
* @param targetConfig The targetConfig to read the `files` and `cmake-include` from.
Expand Down
127 changes: 72 additions & 55 deletions org.lflang/src/org/lflang/generator/c/CUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -553,51 +553,68 @@ static public String triggerRefNested(PortInstance port, String runtimeIndex, St
}

/**
* Copy the 'fileName' from the 'srcDirectory' to the 'destinationDirectory'.
* This function has a fallback search mechanism, where if `fileName` is not
* found in the `srcDirectory`, it will try to find `fileName` via the following procedure:
* 1- Search in LF_CLASSPATH. @see findFile()
* 2- Search in CLASSPATH. @see findFile()
* 3- Search for 'fileName' as a resource.
* That means the `fileName` can be '/path/to/class/resource'. @see java.lang.Class.getResourceAsStream()
*
* @param fileName Name of the file
* @param srcDir Where the file is currently located
* @param dstDir Where the file should be placed
* @return The name of the file in destinationDirectory
*/
public static String copyFileOrResource(String fileName, Path srcDir, Path dstDir) {
// Try to copy the file from the file system.
Path file = findFile(fileName, srcDir);
if (file != null) {
Path target = dstDir.resolve(file.getFileName());
try {
Files.copy(file, target, StandardCopyOption.REPLACE_EXISTING);
return file.getFileName().toString();
} catch (IOException e) {
// Files has failed to copy the file, possibly since
// it doesn't exist. Will try to find the file as a
// resource before giving up.
}
}

// Try to copy the file as a resource.
// If this is missing, it should have been previously reported as an error.
try {
String filenameWithoutPath = fileName;
int lastSeparator = fileName.lastIndexOf(File.separator);
if (lastSeparator > 0) {
filenameWithoutPath = fileName.substring(lastSeparator + 1); // FIXME: brittle. What if the file is in a subdirectory?
}
FileUtil.copyFileFromClassPath(fileName, dstDir.resolve(filenameWithoutPath));
return filenameWithoutPath;
} catch (IOException ex) {
// Ignore. Previously reported as a warning.
System.err.println("WARNING: Failed to find file " + fileName);
}

return "";
}
* Copy the 'fileName' (which also could be a directory name) from the
* 'srcDirectory' to the 'destinationDirectory'. This function has a
* fallback search mechanism, where if `fileName` is not found in the
* `srcDirectory`, it will try to find `fileName` via the following
* procedure:
* 1- Search in LF_CLASSPATH. @see findFile()
* 2- Search in CLASSPATH. @see findFile()
* 3- Search for 'fileName' as a resource. That means the `fileName`
* can be '/path/to/class/resource'. @see java.lang.Class.getResourceAsStream()
*
* @param fileName Name of the file or directory.
* @param srcDir Where the file or directory is currently located.
* @param dstDir Where the file or directory should be placed.
* @return The name of the file or directory in destinationDirectory or an empty string on failure.
*/
public static String copyFileOrResource(String fileName, Path srcDir,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to rename fileName to fileOrDir.

Path dstDir) {
// Try to copy the file or directory from the file system.
Path file = findFileOrDirectory(fileName, srcDir);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename file to path here.

if (file != null) {
Path target = dstDir.resolve(file.getFileName());
try {
if (Files.isDirectory(file)) {
FileUtil.copyDirectory(file, target);
} else if (Files.isRegularFile(file)) {
Files.copy(file, target,
StandardCopyOption.REPLACE_EXISTING);
}
return file.getFileName().toString();
} catch (IOException e) {
// Failed to copy the file or directory, most likely
// because it doesn't exist. Will try to find it as a
// resource before giving up.
}
}

String filenameWithoutPath = fileName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following takes the tail of a path (e.g., foo/bar/baz.lf yields baz.lf; foo/bar yields bar) and just looks for it on the class path. This doesn't seem right to me. I don't think there are any tests for this, and I think the FIXME below alludes to this. I realize this FIXME preexists this PR, but I think we should fix and not ignore it.

int lastSeparator = fileName.lastIndexOf(File.separator);
if (lastSeparator > 0) {
// FIXME: Brittle. What if the file is in a subdirectory?
filenameWithoutPath = fileName.substring(lastSeparator + 1);
}
// Try to copy the file or directory as a resource.
try {
FileUtil.copyFileFromClassPath(fileName,
dstDir.resolve(filenameWithoutPath));
return filenameWithoutPath;
} catch (IOException ex) {
// Will try one more time as a directory
}

try {
FileUtil.copyDirectoryFromClassPath(fileName,
dstDir.resolve(filenameWithoutPath), false);
return filenameWithoutPath;
} catch (IOException ex) {
System.err.println(
"WARNING: Failed to find file or directory " + fileName);
}

return "";
}

//////////////////////////////////////////////////////
//// FIXME: Not clear what the strategy is with the following inner interface.
Expand All @@ -622,23 +639,23 @@ public interface ReportCommandErrors {
}

/**
* Search for a given file name in the given directory.
* Search for a given file or directory name in the given directory.
* If not found, search in directories in LF_CLASSPATH.
* If there is no LF_CLASSPATH environment variable, use CLASSPATH,
* if it is defined.
* The first file found will be returned.
* if it is defined. The first file or directory that is found will
* be returned. Otherwise, null is returned.
*
* @param fileName The file name or relative path + file name
* in plain string format
* @param directory String representation of the director to search in.
* @return A Java file or null if not found
* @param fileName The file or directory name or relative path + name
* as a String.
* @param directory String representation of the directory to search in.
* @return A Java Path or null if not found.
*/
public static Path findFile(String fileName, Path directory) {
public static Path findFileOrDirectory(String fileName, Path directory) {
Path foundFile;

// Check in local directory
foundFile = directory.resolve(fileName);
if (Files.isRegularFile(foundFile)) {
if (Files.exists(foundFile)) {
return foundFile;
}

Expand All @@ -652,7 +669,7 @@ public static Path findFile(String fileName, Path directory) {
String[] paths = classpathLF.split(System.getProperty("path.separator"));
for (String path : paths) {
foundFile = Paths.get(path).resolve(fileName);
if (Files.isRegularFile(foundFile)) {
if (Files.exists(foundFile)) {
return foundFile;
}
}
Expand Down
19 changes: 19 additions & 0 deletions test/C/src/target/Files.lf
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Test the files target property.
*/
target C {
files: ["../include", "include/dummy.h"]
};

preamble {=
#include "include/hello.h"
#include "dummy.h"
=}

main reactor {
reaction(startup) {=
hello_t t;
dummy_t d;
info_print("SUCCESS.");
=}
}
8 changes: 8 additions & 0 deletions test/C/src/target/include/dummy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#ifndef DUMMY_H
#define DUMMY_H

typedef struct dummy_t {
int value;
} dummy_t;

#endif // DUMMY_H