Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Files in include Folder Removed After Compiling with lfc #2203

Open
Depetrol opened this issue Feb 16, 2024 · 9 comments
Open

Files in include Folder Removed After Compiling with lfc #2203

Depetrol opened this issue Feb 16, 2024 · 9 comments
Labels
bug Something isn't working
Milestone

Comments

@Depetrol
Copy link
Collaborator

Issue Description

I encountered this issue when attempting to run the CARLA simulator with Lingua Franca following this instruction. When compiling carla_sync.lf which references include/carla_client.py, carla_client.py within the include folder has been deleted.

This leads to test failures in the PR and should not be the expected behavior.
Untitled

Temporary Fix

Move files in the include folder elsewhere, and change the file references. In the case of CARLA simulator, move include/carla_client.py to the parent/workspace folder and change carla_sync.lf :

target Python {
  files: carla_client.py
}

Proposed Fix

Ensure files in the include folder are retained after compilation, or issue a warning during the compilation process to notify the user that files in the include folder might be removed.

@lhstrh
Copy link
Member

lhstrh commented Feb 16, 2024

Thanks for the bug report, @Depetrol. We will work on this and push a fix.

@cmnrd cmnrd added this to the 0.7.0 milestone Feb 16, 2024
@cmnrd cmnrd added the bug Something isn't working label Feb 16, 2024
@petervdonovan
Copy link
Collaborator

I think that this is probably because we generate some headers and keep them in a user-facing (not in src-gen) directory. When the headers are re-generated, we probably delete the whole directory.

Probably the right way to solve this is to rename the directory for the generated headers from include to include-gen to reduce the chance of getting confused about whether the user is supposed to edit the files in that directory. This should have been done when I introduced the mechanism for generating headers because the potential for it to conflict with the user's source files is clear.

@lhstrh
Copy link
Member

lhstrh commented Feb 17, 2024

I like the include-gen solution you propose.

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 20, 2024

Oh, so the problem is that this example has an include directory next to the src directory, which violates our assumption that all sources are located in src.

Renaming "our" include directory is not easy. It is generated automatically by cmake when installing the runtime lib (and potentially other libs and reactor headers). We could try to rename this (I am sure there is a hack for that), but the bin, lib, include, share directory structure is established on POSIX systems and we would break with that.

I see two solutions:

  1. Clearly state that all source files should be located within src and try to enforce this in our validation as far as we can.
  2. Introduce an install directory. Alternatively, we could also use the build-type and use a debug and release directory, which some build-tool or IDEs do. Then we would let cmake install within this directory. But this would change the location of our binaries, which might cause problems elsewhere.

@petervdonovan
Copy link
Collaborator

This is complicated by the fact that the C target does not work in the same was as the C++ target. In the C target, some headers are generated by the code generator lfc and not by CMake, so actually we do have control over where those headers go. Those headers are intended to be used in the files that appear in src, which is probably pretty different from what the include directory is for in the case of the C++ target.

I'm not sure if it makes sense to try to unify the behavior of the two targets.

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 21, 2024

Ok, those files should go into src-gen. We can just create a nested include directory and make this part of the include path. The C++ also copies an LF specific header file into src-gen.

Those headers are intended to be used in the files that appear in src, which is probably pretty different from what the include directory is for in the case of the C++ target.

Yes, in the C++ target the include directory is part of the compilation artifacts (like the bin directory). I thought the C target actually does the same, but we should investigate more closely.

I'm not sure if it makes sense to try to unify the behavior of the two targets.

I think we should, it's just unclear what the best solution would be.

@petervdonovan
Copy link
Collaborator

petervdonovan commented Feb 21, 2024

Ok, those files should go into src-gen. We can just create a nested include directory and make this part of the include path. The C++ also copies an LF specific header file into src-gen.

This works for me. I'll go along with whichever directory layout we can get everyone on board with.

@lhstrh
Copy link
Member

lhstrh commented Jul 4, 2024

lf-lang/rfcs#11 discusses project layout/directory structure.

@cmnrd
Copy link
Collaborator

cmnrd commented Jul 4, 2024

Yes, we also discussed the overlap between this issue and the new RFC in the last developer meeting. However, we resolved to focus lf-lang/rfcs#11 on where we place LF code, and not where and which directories we generate. This is a different concern that should be handled in another RFC.

@lhstrh lhstrh assigned cmnrd and unassigned petervdonovan Aug 5, 2024
@lhstrh lhstrh modified the milestones: 0.9.0, 0.10.0 Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants