Skip to content

Commit 324e997

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.10) (#24822)
This PR cherry-picks changes from the original PR #24821 to branch 24.10. --- #### 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 753f61d commit 324e997

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
@@ -511,11 +511,24 @@ public boolean isJarProject() {
511511

512512
@Override
513513
public String buildFolder() {
514-
if (projectBuildDir.startsWith(projectBasedir.toString())) {
515-
return projectBaseDirectory().relativize(Paths.get(projectBuildDir))
514+
Path buildDir = Paths.get(projectBuildDir);
515+
// buildFolder() is consumed as a path relative to the project folder
516+
// (new File(npmFolder, buildFolder)), so always return it relative to
517+
// the project basedir. A build dir outside basedir then yields a "../"
518+
// path. Returning the absolute path would instead append it to the
519+
// project folder and point outside the build dir.
520+
if (!buildDir.isAbsolute()) {
521+
return projectBuildDir;
522+
}
523+
// relativize() requires both paths to be absolute; basedir is always
524+
// absolute in a real build, toAbsolutePath() only guards exotic setups.
525+
try {
526+
return projectBaseDirectory().toAbsolutePath().relativize(buildDir)
516527
.toString();
528+
} catch (IllegalArgumentException e) {
529+
// Different filesystem roots (e.g. on Windows): cannot relativize.
530+
return projectBuildDir;
517531
}
518-
return projectBuildDir;
519532
}
520533

521534
@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
@@ -19,6 +19,7 @@ package com.vaadin.flow.gradle
1919
import com.vaadin.flow.internal.JacksonUtils
2020
import com.vaadin.flow.internal.StringUtil
2121
import com.vaadin.flow.server.InitParameters
22+
import com.vaadin.flow.server.frontend.TaskUpdateSettingsFile
2223
import org.gradle.testkit.runner.BuildResult
2324
import org.junit.Test
2425
import java.io.File
@@ -281,5 +282,59 @@ class MiscMultiModuleTest : AbstractGradleTest() {
281282
assertContains(result2.output, "Reusing configuration cache")
282283
}
283284

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

285340
}

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
@@ -263,10 +263,19 @@ internal class GradlePluginAdapter private constructor(
263263

264264
override fun buildFolder(): String {
265265
val projectBuildDir = config.projectBuildDir.get()
266-
if (projectBuildDir.startsWith(projectDir.toString())) {
267-
return File(projectBuildDir).relativeTo(projectDir).toString()
266+
val buildDirFile = File(projectBuildDir)
267+
// buildFolder() is consumed as a path relative to the project folder
268+
// (new File(npmFolder, buildFolder)), so always return it relative to
269+
// projectDir. When the build dir is outside projectDir this yields a "../"
270+
// path. Returning the absolute path would instead append it to the project
271+
// folder and point outside the build dir.
272+
// relativeToOrNull() needs a shared root; projectDir is always absolute
273+
// in a real build, absoluteFile only guards exotic setups.
274+
return when {
275+
!buildDirFile.isAbsolute -> projectBuildDir
276+
else -> buildDirFile.relativeToOrNull(projectDir.absoluteFile)
277+
?.toString() ?: projectBuildDir
268278
}
269-
return projectBuildDir
270279
}
271280

272281
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
@@ -698,11 +698,24 @@ public boolean isJarProject() {
698698

699699
@Override
700700
public String buildFolder() {
701-
if (projectBuildDir.startsWith(projectBasedir.toString())) {
702-
return projectBaseDirectory().relativize(Paths.get(projectBuildDir))
701+
Path buildDir = Paths.get(projectBuildDir);
702+
// buildFolder() is consumed as a path relative to the project folder
703+
// (new File(npmFolder, buildFolder)), so always return it relative to
704+
// the project basedir. A build dir outside basedir then yields a "../"
705+
// path. Returning the absolute path would instead append it to the
706+
// project folder and point outside the build dir.
707+
if (!buildDir.isAbsolute()) {
708+
return projectBuildDir;
709+
}
710+
// relativize() requires both paths to be absolute; basedir is always
711+
// absolute in a real build, toAbsolutePath() only guards exotic setups.
712+
try {
713+
return projectBaseDirectory().toAbsolutePath().relativize(buildDir)
703714
.toString();
715+
} catch (IllegalArgumentException e) {
716+
// Different filesystem roots (e.g. on Windows): cannot relativize.
717+
return projectBuildDir;
704718
}
705-
return projectBuildDir;
706719
}
707720

708721
@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
@@ -19,6 +19,7 @@
1919
import java.io.File;
2020
import java.io.IOException;
2121
import java.nio.charset.StandardCharsets;
22+
import java.nio.file.Path;
2223
import java.nio.file.Paths;
2324
import java.util.Arrays;
2425
import java.util.Collections;
@@ -59,6 +60,7 @@
5960
import static com.vaadin.flow.server.Constants.VAADIN_SERVLET_RESOURCES;
6061
import static com.vaadin.flow.server.Constants.VAADIN_WEBAPP_RESOURCES;
6162
import static com.vaadin.flow.server.frontend.FrontendUtils.TOKEN_FILE;
63+
import static org.junit.Assert.assertEquals;
6264

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

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
@@ -295,8 +295,15 @@ default File frontendOutputDirectory() {
295295

296296
/**
297297
* The folder where everything is built into.
298-
*
299-
* @return build folder
298+
* <p>
299+
* The returned value must be a path <em>relative</em> to the project root
300+
* (the {@link #npmFolder() npm folder}), because consumers resolve it
301+
* against that folder, e.g. {@code new File(npmFolder(), buildFolder())}.
302+
* When the build directory lives outside the project root this means
303+
* returning a {@code "../"} path; returning an absolute path would instead
304+
* be appended under the project root and point to the wrong location.
305+
*
306+
* @return build folder, relative to the project root
300307
*/
301308
String buildFolder();
302309

0 commit comments

Comments
 (0)