Skip to content

Commit e2c06da

Browse files
vaadin-botArtur-ingokegelmcollovati
authored
fix: Write frontend build files into build dir when it is outside project dir (#24805) (CP: 25.0) (#24821) (CP: 24.9) (#24823)
This PR cherry-picks changes from the original PR #24821 to branch 24.9. --- #### Original PR description > This PR cherry-picks changes from the original PR #24805 to branch 25.0. > --- > #### Original PR description > > ## Description > > > > When the build dir is relocated outside the project dir, vaadinPrepareFrontend wrote vaadin-dev-server-settings.json and the dev-bundle output paths under the source tree. buildFolder() is now always relative to projectDir. > > > > Fixes #24804 > > > > ## Type of change > > > > - [ x] Bugfix > > - [ ] Feature > > > > ## Checklist > > > > - [x ] I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/ > > - [x ] I have added a description following the guideline. > > - [ x] The issue is created in the corresponding repository and I have referenced it. > > - [ x] I have added tests to ensure my change is effective and works as intended. > > - [ x] New and existing tests are passing locally with my change. > > - [ x] I have performed self-review and corrected misspellings. > > > > > Co-authored-by: Artur Signell <artur@vaadin.com> Co-authored-by: Ingo Kegel <ingo.kegel@ej-technologies.com> Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent 1887035 commit e2c06da

6 files changed

Lines changed: 179 additions & 11 deletions

File tree

flow-plugins/flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/BuildDevBundleMojo.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -504,11 +504,24 @@ public boolean isJarProject() {
504504

505505
@Override
506506
public String buildFolder() {
507-
if (projectBuildDir.startsWith(projectBasedir.toString())) {
508-
return projectBaseDirectory().relativize(Paths.get(projectBuildDir))
507+
Path buildDir = Paths.get(projectBuildDir);
508+
// buildFolder() is consumed as a path relative to the project folder
509+
// (new File(npmFolder, buildFolder)), so always return it relative to
510+
// the project basedir. A build dir outside basedir then yields a "../"
511+
// path. Returning the absolute path would instead append it to the
512+
// project folder and point outside the build dir.
513+
if (!buildDir.isAbsolute()) {
514+
return projectBuildDir;
515+
}
516+
// relativize() requires both paths to be absolute; basedir is always
517+
// absolute in a real build, toAbsolutePath() only guards exotic setups.
518+
try {
519+
return projectBaseDirectory().toAbsolutePath().relativize(buildDir)
509520
.toString();
521+
} catch (IllegalArgumentException e) {
522+
// Different filesystem roots (e.g. on Windows): cannot relativize.
523+
return projectBuildDir;
510524
}
511-
return projectBuildDir;
512525
}
513526

514527
@Override

flow-plugins/flow-gradle-plugin/src/functionalTest/kotlin/com/vaadin/gradle/MiscMultiModuleTest.kt

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ package com.vaadin.flow.gradle
1111
import com.vaadin.flow.internal.JacksonUtils
1212
import com.vaadin.flow.internal.StringUtil
1313
import com.vaadin.flow.server.InitParameters
14+
import com.vaadin.flow.server.frontend.TaskUpdateSettingsFile
1415
import org.gradle.testkit.runner.BuildResult
1516
import org.junit.Test
1617
import java.io.File
@@ -273,5 +274,59 @@ class MiscMultiModuleTest : AbstractGradleTest() {
273274
assertContains(result2.output, "Reusing configuration cache")
274275
}
275276

277+
/**
278+
* When the build directory is relocated outside the module project directory, the plugin must still write
279+
* vaadin-dev-server-settings.json into that build directory.
280+
*/
281+
@Test
282+
fun `build dir relocated outside project dir writes settings into build dir`() {
283+
testProject.settingsFile.writeText("include 'web'")
284+
testProject.buildFile.writeText("""
285+
plugins {
286+
id 'java'
287+
id 'com.vaadin.flow' apply false
288+
}
289+
allprojects {
290+
repositories {
291+
mavenLocal()
292+
mavenCentral()
293+
maven { url = 'https://maven.vaadin.com/vaadin-prereleases' }
294+
}
295+
}
296+
project(':web') {
297+
apply plugin: 'war'
298+
apply plugin: 'com.vaadin.flow'
299+
300+
dependencies {
301+
implementation("com.vaadin:flow:$flowVersion")
302+
}
303+
304+
// Relocate the build dir outside the :web project dir (here a sibling of
305+
// the module, like builds that share one top-level build/ folder). The
306+
// resulting value is absolute and is not a sub-directory of projectDir.
307+
layout.buildDirectory = file("${'$'}{rootDir}/custom-build")
308+
309+
vaadin {
310+
eagerServerLoad = false
311+
}
312+
}
313+
""".trimIndent())
314+
testProject.newFolder("web")
315+
316+
testProject.build("web:vaadinPrepareFrontend")
317+
318+
// The settings file must be written into the relocated build dir...
319+
val settingsInBuildDir = File(testProject.dir,
320+
"custom-build/${TaskUpdateSettingsFile.DEV_SETTINGS_FILE}")
321+
expect(true, "$settingsInBuildDir should exist") { settingsInBuildDir.exists() }
322+
323+
// ...and must not leak into a junk tree under the :web source directory.
324+
val leaked = File(testProject.dir, "web").walkTopDown()
325+
.filter { it.name == TaskUpdateSettingsFile.DEV_SETTINGS_FILE }
326+
.toList()
327+
expect(emptyList<File>(),
328+
"vaadin-dev-server-settings.json leaked under the web source dir: $leaked") { leaked }
329+
}
330+
276331

277332
}

flow-plugins/flow-gradle-plugin/src/main/kotlin/com/vaadin/gradle/GradlePluginAdapter.kt

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,19 @@ internal class GradlePluginAdapter private constructor(
256256

257257
override fun buildFolder(): String {
258258
val projectBuildDir = config.projectBuildDir.get()
259-
if (projectBuildDir.startsWith(projectDir.toString())) {
260-
return File(projectBuildDir).relativeTo(projectDir).toString()
259+
val buildDirFile = File(projectBuildDir)
260+
// buildFolder() is consumed as a path relative to the project folder
261+
// (new File(npmFolder, buildFolder)), so always return it relative to
262+
// projectDir. When the build dir is outside projectDir this yields a "../"
263+
// path. Returning the absolute path would instead append it to the project
264+
// folder and point outside the build dir.
265+
// relativeToOrNull() needs a shared root; projectDir is always absolute
266+
// in a real build, absoluteFile only guards exotic setups.
267+
return when {
268+
!buildDirFile.isAbsolute -> projectBuildDir
269+
else -> buildDirFile.relativeToOrNull(projectDir.absoluteFile)
270+
?.toString() ?: projectBuildDir
261271
}
262-
return projectBuildDir
263272
}
264273

265274
override fun postinstallPackages(): List<String> =

flow-plugins/flow-maven-plugin/src/main/java/com/vaadin/flow/plugin/maven/FlowModeAbstractMojo.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -691,11 +691,24 @@ public boolean isJarProject() {
691691

692692
@Override
693693
public String buildFolder() {
694-
if (projectBuildDir.startsWith(projectBasedir.toString())) {
695-
return projectBaseDirectory().relativize(Paths.get(projectBuildDir))
694+
Path buildDir = Paths.get(projectBuildDir);
695+
// buildFolder() is consumed as a path relative to the project folder
696+
// (new File(npmFolder, buildFolder)), so always return it relative to
697+
// the project basedir. A build dir outside basedir then yields a "../"
698+
// path. Returning the absolute path would instead append it to the
699+
// project folder and point outside the build dir.
700+
if (!buildDir.isAbsolute()) {
701+
return projectBuildDir;
702+
}
703+
// relativize() requires both paths to be absolute; basedir is always
704+
// absolute in a real build, toAbsolutePath() only guards exotic setups.
705+
try {
706+
return projectBaseDirectory().toAbsolutePath().relativize(buildDir)
696707
.toString();
708+
} catch (IllegalArgumentException e) {
709+
// Different filesystem roots (e.g. on Windows): cannot relativize.
710+
return projectBuildDir;
697711
}
698-
return projectBuildDir;
699712
}
700713

701714
@Override

flow-plugins/flow-maven-plugin/src/test/java/com/vaadin/flow/plugin/maven/PrepareFrontendMojoTest.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.io.File;
1212
import java.io.IOException;
1313
import java.nio.charset.StandardCharsets;
14+
import java.nio.file.Path;
1415
import java.nio.file.Paths;
1516
import java.util.Arrays;
1617
import java.util.Collections;
@@ -51,6 +52,7 @@
5152
import static com.vaadin.flow.server.Constants.VAADIN_SERVLET_RESOURCES;
5253
import static com.vaadin.flow.server.Constants.VAADIN_WEBAPP_RESOURCES;
5354
import static com.vaadin.flow.server.frontend.FrontendUtils.TOKEN_FILE;
55+
import static org.junit.Assert.assertEquals;
5456

5557
public class PrepareFrontendMojoTest {
5658
@Rule
@@ -270,4 +272,73 @@ private void assertPackageJsonContent() throws IOException {
270272
"@rollup/plugin-replace", "rollup-plugin-brotli",
271273
"vite-plugin-checker");
272274
}
275+
276+
@Test
277+
public void buildFolder_buildDirInsideBasedir_returnsRelativeName()
278+
throws Exception {
279+
ReflectionUtils.setVariableValueInObject(mojo, "projectBuildDir",
280+
new File(projectBase, "target").getAbsolutePath());
281+
282+
assertEquals("target", mojo.buildFolder());
283+
}
284+
285+
// buildFolder() is consumed as new File(npmFolder, buildFolder()), so the
286+
// four absolute/relative combinations of basedir and build dir must all
287+
// resolve back to the real build dir, never a nested junk path under the
288+
// source tree. Only the absolute/absolute combination occurs in a real
289+
// Maven build; the others guard exotic setups.
290+
291+
@Test
292+
public void buildFolder_baseAbsolute_buildAbsolute_resolvesToBuildDir()
293+
throws Exception {
294+
File basedir = projectBase; // absolute
295+
File buildDir = new File(projectBase.getParentFile(), "custom-build");
296+
assertBuildFolderResolvesTo(basedir, buildDir.getAbsolutePath(),
297+
buildDir);
298+
}
299+
300+
@Test
301+
public void buildFolder_baseAbsolute_buildRelative_resolvesToBuildDir()
302+
throws Exception {
303+
File basedir = projectBase; // absolute
304+
String buildDir = "../custom-build"; // relative to basedir
305+
assertBuildFolderResolvesTo(basedir, buildDir,
306+
new File(basedir, buildDir));
307+
}
308+
309+
@Test
310+
public void buildFolder_baseRelative_buildAbsolute_resolvesToBuildDir()
311+
throws Exception {
312+
File basedir = new File("relative-base");
313+
File buildDir = new File(projectBase.getParentFile(), "custom-build");
314+
assertBuildFolderResolvesTo(basedir, buildDir.getAbsolutePath(),
315+
buildDir);
316+
}
317+
318+
@Test
319+
public void buildFolder_baseRelative_buildRelative_resolvesToBuildDir()
320+
throws Exception {
321+
File basedir = new File("relative-base");
322+
String buildDir = "../custom-build"; // relative to basedir
323+
assertBuildFolderResolvesTo(basedir, buildDir,
324+
new File(basedir, buildDir));
325+
}
326+
327+
private void assertBuildFolderResolvesTo(File basedir,
328+
String projectBuildDir, File expectedBuildDir) throws Exception {
329+
// npmFolder always equals basedir in a real project.
330+
ReflectionUtils.setVariableValueInObject(mojo, Constants.NPM_TOKEN,
331+
basedir);
332+
ReflectionUtils.setVariableValueInObject(mojo, "projectBasedir",
333+
basedir);
334+
ReflectionUtils.setVariableValueInObject(mojo, "projectBuildDir",
335+
projectBuildDir);
336+
337+
File resolved = new File(mojo.npmFolder(), mojo.buildFolder());
338+
assertEquals(normalize(expectedBuildDir), normalize(resolved));
339+
}
340+
341+
private static Path normalize(File file) {
342+
return file.getAbsoluteFile().toPath().normalize();
343+
}
273344
}

flow-plugins/flow-plugin-base/src/main/java/com/vaadin/flow/plugin/base/PluginAdapterBase.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,15 @@ default File frontendOutputDirectory() {
288288

289289
/**
290290
* The folder where everything is built into.
291-
*
292-
* @return build folder
291+
* <p>
292+
* The returned value must be a path <em>relative</em> to the project root
293+
* (the {@link #npmFolder() npm folder}), because consumers resolve it
294+
* against that folder, e.g. {@code new File(npmFolder(), buildFolder())}.
295+
* When the build directory lives outside the project root this means
296+
* returning a {@code "../"} path; returning an absolute path would instead
297+
* be appended under the project root and point to the wrong location.
298+
*
299+
* @return build folder, relative to the project root
293300
*/
294301
String buildFolder();
295302

0 commit comments

Comments
 (0)