prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
15.89k stars 5.32k forks source link

mvn clean install adds files to presto-main/src #22685

Open elharo opened 4 months ago

elharo commented 4 months ago

I've noticed that recently these new files keep showing up in my repo:

Untracked files: (use "git add ..." to include in what will be committed) presto-main/src/main/resources/webapp/dev/query_viewer.js presto-main/src/main/resources/webapp/dist/

I think these belong instead in the generated-sources directory (or something like that; still need to fully debug and research this). I suspect they've been added as a result of #22645

yhwang commented 4 months ago

These untracked files are from the #22645 (but they were not added by the PR, the PR removed them from the sources):

presto-main/src/main/resources/webapp/dev/query_viewer.js
presto-main/src/main/resources/webapp/dist/

@ZacBlanco also informed me about this issue The possible solutions are:

  1. Add these files into .gitigonre
  2. Modify the webpack to generate compiled JS to presto-main/target directory

For solution 2, it may need to tweak the webpack config to cover the dev scenarios. Need to make sure yarn watch, yarn serve work with the new JS location.

elharo commented 4 months ago

1 is not really a solution. #2 is a maybe. In the past I've seen these sorts of tools use a generated-sources directory though at the moment, I can't find any docs about that. However we definitely should not generate anything into src/main. We need to put generated code somewhere mvn clean will delete it.

yhwang commented 4 months ago

@elharo

We need to put generated code somewhere mvn clean will delete it

That's option 2.

elharo commented 4 months ago

had to look in the maven-compiler-plugin source code but it likely belongs in

${project.build.directory}/generated-sources/

yhwang commented 4 months ago

FYI, Presto is following the Maven stadnard directory layout: https://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html

elharo commented 4 months ago

Yes, and nothing in that layout is for generated code.

yhwang commented 4 months ago

for .js files, they are equal to .class which would go to target/classes