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

[Improvement]: Update the project which is in ToolContext after each tool config execute #42599

Open
lnash94 opened this issue Apr 19, 2024 · 3 comments
Assignees
Labels
Area/CLI Ballerina CLI related issues. Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) Type/Improvement userCategory/Compilation

Comments

@lnash94
Copy link
Member

lnash94 commented Apr 19, 2024

Description

We've observed that the Ballerina.toml file isn't being updated with each tool configuration run. This becomes problematic when there are interdependent tool configurations relying on the details updated in the TOML file.

ex:
Let's say we have configurations for tools A and B in Ballerina.toml. Tool B depends on changes made by Tool A. However, with the current existing tool context, we received the initial Ballerina.toml file, which has not been updated after the changes made by Tool A.

Describe your problem(s)

No response

Describe your solution(s)

No response

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@ballerina-bot ballerina-bot added needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Apr 19, 2024
@gayaldassanayake gayaldassanayake moved this to Planned for Sprint in Ballerina Team Main Board Apr 19, 2024
@gayaldassanayake gayaldassanayake added Area/CLI Ballerina CLI related issues. Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) and removed needTriage The issue has to be inspected and labeled manually labels Apr 19, 2024
@gayaldassanayake
Copy link
Contributor

Had an offline discussion with @azinneera. The suggested requirement can be facilitated by loading the project after running each tool, before the next tool execution.

However, the original intended design was to run the tasks independently of each other. Furthermore, we do not guarantee the order of execution of the tools. So we decided not to follow through with this design unless there is no feasible alternative approach.

The suggested alternative approach is to use the tool cache to record if the tool is already executed. @lnash94 please check if this is feasible from your end.
The other method is to reload the project from the tool side.

@gayaldassanayake gayaldassanayake self-assigned this May 6, 2024
@TharmiganK
Copy link
Contributor

@gayaldassanayake @azinneera Let me explain the actual problem here.

We can have multiple OpenAPI tool configurations in the Ballerina.toml where each one can update the existing Ballerina.toml file with some Java platform dependencies.

We are checking the existence of this dependency by getting the manifest from the project, and if it already there, we are skipping the toml update.

Since we are not loading the project after updating the Ballerina.toml, the above check returns false for the second time and it ends up in updating the Ballerina.toml with two same dependencies. This makes the build fail because of the duplicate entries of the same dependency.

Actually, we can read the Ballerina.toml file in our tool to check the dependency, rather than using the manifest. But we used that because we don't want to use a toml parser or some toml parsing logic by ourself. (This is a common use case in persist also, but currently they are reading the file in their tool side)

So we are not concerned about the order of the tool execution, but we expect the project to be updated with the previous tool changes. If that is not a viable solution, can you suggest a different approach to read and update the Ballerina.toml via some lang/tool functions?

@azinneera azinneera moved this from Planned for Sprint to BackLog in Ballerina Team Main Board May 16, 2024
@azinneera
Copy link
Contributor

@TharmiganK you can load the project within the tool by using BuildProject.load(toolContext.currentPackage().project().sourceRoot()) and then load the manifest to check if the entry is already there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/CLI Ballerina CLI related issues. Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) Type/Improvement userCategory/Compilation
Projects
None yet
Development

No branches or pull requests

5 participants