Bug 492819 - Pomless builds with deep folder structure
Summary: Pomless builds with deep folder structure
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Tycho (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Christoph Laeubrich CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2016-05-02 09:43 EDT by Max Bureck CLA
Modified: 2021-04-28 16:51 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Max Bureck CLA 2016-05-02 09:43:36 EDT
Hello,

We have several repositories built with Tycho, a couple of them using the POM-less build. These repositories all follow the Eclipse repository layout convention of a "plugins" (or "bundles") and a "features", and some a "tests" directory. We have a central parent POM in the root directory (sometimes itself a child of another parent in a "releng" directory).

To make POM-less builds work, we need a POM file in "plugins", "features" and "tests" folders, simply configuring the central parent POM as their parent. These almost empty POM files could be avoided if org.eclipse.tycho.pomless.TychoModelReader#findParent(File) would either 
* Look up the parent's parent if the parent folder is no maven project
* Allow configuration of now many levels the function may go up to look for a parent project, if the next parent is not a maven project

I would argue that this project layout is common enough, so that support for this kind of parent lookup makes sense.

The only downside that I see is that without explicit configuration (as suggested in the second variant) this behavior may be surprising if not known.
Comment 1 Eclipse Genie CLA 2016-05-16 01:47:26 EDT
New Gerrit change created: https://git.eclipse.org/r/72796
Comment 2 Martin Schreiber CLA 2016-05-16 01:51:04 EDT
(In reply to Eclipse Genie from comment #1)
> New Gerrit change created: https://git.eclipse.org/r/72796

There had been discussions whether searching up in the directory structure or not is a good idea. This gerrit change contains the code where the directory structure is searing up level by level to find a parent pom. It will also log (if debug is enabled) which parent pom it is taking into account.
Comment 3 Jan Sievers CLA 2016-05-17 03:24:25 EDT
(In reply to Martin Schreiber from comment #2)
> (In reply to Eclipse Genie from comment #1)
> > New Gerrit change created: https://git.eclipse.org/r/72796
> 
> There had been discussions whether searching up in the directory structure
> or not is a good idea.

do you have references to that discussion?
I think this behaviour may have unexpected side-effects, e.g. the build behaves differently depending on whether somewhere up in the directory tree at arbitrary level a pom can be found. That means the build is no longer fully self-contained and can "leak" up the file system hierarchy.

I don't know if there is a way to configure build extensions in .mvn/extensions.xml , but if there is I would prefer to make the parent pom (relative) path somehow explicit there (and if nothing is configured, the current behaviour stays unchanged)
Comment 4 Max Bureck CLA 2016-05-17 05:45:14 EDT
From a quick glance into the extension mechanism it does not seem possible to add own configuration to the .mvn/extensions.xml file, but maybe I am wrong. But if this is not possible, maybe it is possible to find the reactor root and manually read a different/additional configuration file like ".mvn/pomless.xml".

If the parent lookup should be be configured, then which configuration option(s) should be provided?

1) A fixed parent for all projects missing a direct parent (e.g. relative path from the reactor root)?

2) An an exact amount of levels to go up in the directory hierarchy to find the parent? 

3) A maximum level of levels to go up in the directory hierarchy?

4) A path (relative down from reactor root) that should not be searched further up from?

5) A combination of the above?

In my opinion option 1) and 2) restrict the repository layout too much, although in our case this would be sufficient. Option 3), 4), or a combination of both allow a great deal of flexibility. The default value for 3) could be 1 (meaning only direct super directory will be searched). The default value for 4) could be the reactor root, so that no parent lookup would leave the root directory and pick up "external" parents.

But all of this would need the information of the root directory, the question would be how to look this up from the pomless extension.
Comment 5 Martin Schreiber CLA 2016-05-17 06:33:05 EDT
(In reply to Jan Sievers from comment #3)
> (In reply to Martin Schreiber from comment #2)
> > (In reply to Eclipse Genie from comment #1)
> > > New Gerrit change created: https://git.eclipse.org/r/72796
> > 
> > There had been discussions whether searching up in the directory structure
> > or not is a good idea.
> 
> do you have references to that discussion?
> I think this behaviour may have unexpected side-effects, e.g. the build
> behaves differently depending on whether somewhere up in the directory tree
> at arbitrary level a pom can be found. That means the build is no longer
> fully self-contained and can "leak" up the file system hierarchy.
> 

I thought there had been discussions on the user-group but found only this bug which is related: Bug 482861
Comment 6 Martin Schreiber CLA 2016-05-17 06:58:12 EDT
(In reply to Max Bureck from comment #4)
> From a quick glance into the extension mechanism it does not seem possible
> to add own configuration to the .mvn/extensions.xml file, but maybe I am
> wrong. But if this is not possible, maybe it is possible to find the reactor
> root and manually read a different/additional configuration file like
> ".mvn/pomless.xml".
> 
> If the parent lookup should be be configured, then which configuration
> option(s) should be provided?
> 
> 1) A fixed parent for all projects missing a direct parent (e.g. relative
> path from the reactor root)?
> 
> 2) An an exact amount of levels to go up in the directory hierarchy to find
> the parent? 
> 
> 3) A maximum level of levels to go up in the directory hierarchy?
> 
> 4) A path (relative down from reactor root) that should not be searched
> further up from?
> 
> 5) A combination of the above?
> 
> In my opinion option 1) and 2) restrict the repository layout too much,
> although in our case this would be sufficient. Option 3), 4), or a
> combination of both allow a great deal of flexibility. The default value for
> 3) could be 1 (meaning only direct super directory will be searched). The
> default value for 4) could be the reactor root, so that no parent lookup
> would leave the root directory and pick up "external" parents.
> 
> But all of this would need the information of the root directory, the
> question would be how to look this up from the pomless extension.

I would not go for an extra config file which would only contain that information. 
How about a system property, e.g. "-DparentPomSearchLevels=3" which would search up 3 levels (at most) to find the parent pom. If not specified it would expect the pom in the parent directory as it is doing now.
Comment 7 Max Bureck CLA 2016-05-17 07:30:29 EDT
I would not consider a system property ideal. The best state of a repository is to be able to simply drop into the root folder and execute "mvn install" or "mvn package" to build with the default configuration. If the configuration cannot be placed in configuration files somewhere, manual adjustments of parameters, or external mechanisms (like platform specific shell scripts invoking the builds) are needed. Both variants feel like kludges. In my opinion a system property can only be an alternative possibility to a configuration file, but not be the only possibility for configuration. I doesn't matter which configuration file this is, as long as it is part of the repository and in the best case does not have to be adjusted for every project individually.
Comment 8 Jan Sievers CLA 2016-05-17 09:10:42 EDT
(In reply to Max Bureck from comment #4)
> From a quick glance into the extension mechanism it does not seem possible
> to add own configuration to the .mvn/extensions.xml file, but maybe I am
> wrong. 

yes, looks like there are no configuration parameters in .mvn/extensions.xml right now: https://issues.apache.org/jira/browse/MNG-5897

for reference, user-configurable core extensions were originally introduced with https://issues.apache.org/jira/browse/MNG-5771

I would prefer not to use system properties as these would make the build non-self-contained and a simple 'mvn clean install' would no longer just work (same argument as above). 'mvn clean install' must always work reliably, this is a hard requirement for any maven build from my point of view.

OTOH I would prefer not to add a tycho-specific configuration file; rather try to fix MNG-5897 or as a stopgap at least use .mvn/extensions.xml in the Tycho pomless extension to read configuration

I prefer to keep things simple and straightforward, too much magic would definitely do more harm than good here. (Debugging maven POM inheritance problems can be quite hard and non-obvious behaviour of which parent is taken from where has cost me hours in the past)

Proposal:

Make the pom parent.relativePath POM element (as per [1]) globally configurable for a given reactor in a way that is stored with the project sources (preferably in .mvn/extensions.xml).

[1] https://maven.apache.org/pom.html#Inheritance
Comment 9 Max Bureck CLA 2016-05-17 09:40:42 EDT
It looks like the cleanest solution would be to wait for MNG-5897 being fixed.

To simply allow defining parent.relativePath centrally is certainly a simple solution, but it is not very flexible. It needs all projects to have the same relative path to the central parent. Nevertheless, for the common repository layout that we (and a vast amount of Eclipse projects) have, this solution would be sufficient.
Comment 10 Christoph Laeubrich CLA 2019-08-24 10:32:13 EDT
I have a implemented a working solution for these "structured builds", will provide a patch as soon as https://git.eclipse.org/r/#/c/148033/  is merged since the solution builds on top of this change.

It does not introduce generic configuration of parent-pom location, but instead generates a "pomless-parent-pom" for these well known folders that then references the (valid) subfolders as modules.

one can the in the root pom reference these folders simply by:

	<modules>
		<module>bundles</module>
		<module>features</module>
		<module>tests</module>
		<module>releng</module>
	</modules>

and required intermediate parent poms are generated by tycho-pomless on the fly.

You can even reference this "on-the-fly" parent if you like (e.g. you are using a pom xml inside one of the bundles) the following way:

	<parent>
		<groupId>com.vogella.tycho</groupId>
		<artifactId>bundles</artifactId>
		<version>1.0.0-SNAPSHOT</version>
		<relativePath>../pom.tycho</relativePath>
	</parent>

	<artifactId>com.vogella.tycho.help</artifactId>
	<packaging>eclipse-plugin</packaging>

this should cover most common use-case when building structured-builds pomless.
Comment 11 Eclipse Genie CLA 2019-08-26 04:41:22 EDT
New Gerrit change created: https://git.eclipse.org/r/148324
Comment 12 Christoph Laeubrich CLA 2019-08-26 04:50:15 EDT
This reduces the required poms to just one root pom-file.
Comment 13 Max Bureck CLA 2019-08-26 05:19:01 EDT
Interesting solution, which is probably sufficient for the most common layouts. 

However, it would be more flexible if the user could override "tycho.pomless.aggregator.names" in a root pom and not rely on a system property that is statically read once. But I don't have any knowledge of the lifecycle of mavens polyglot machinery and Tycho's Mapping architecture, so maybe this cannot be done.
Comment 14 Christoph Laeubrich CLA 2019-08-26 06:01:51 EDT
(In reply to Max Bureck from comment #13)
> Interesting solution, which is probably sufficient for the most common
> layouts. 

Thats the idea to support at leas a common strucutured layout, with some restrictions (as is that the pom.xml must reside in the parent folder)

> However, it would be more flexible if the user could override
> "tycho.pomless.aggregator.names" in a root pom and not rely on a system
> property that is statically read once. But I don't have any knowledge of the
> lifecycle of mavens polyglot machinery and Tycho's Mapping architecture, so
> maybe this cannot be done.

It can be done, but very very limited, we can read the properties from the parent BUT as we are in "model generation phase" we can't infere e.g. properties defined in parents-parent pom, defined on comandline (mvn -D) or expanded (afaik), so I decided to at least provide a property to support more layouts that we currently can think of, but it should not be the common (or even recommended) to configure this.
Comment 15 Max Bureck CLA 2019-08-27 05:08:30 EDT
I think a static configuration would be sufficient for almost all repos. But you are right: the limitations you mentioned are against the expectations how maven works and would probably surprise users, even if these restrictions were documented.

One could side-step the issue by allowing the configuration only in an own config file, e.g. in the reactor's root .mvn directory. On the other hand, another custom config file seems like a hollow victory, since we want fewer config files and not another one.

At least this change produces "soft pressure" to use naming conventions making it easier to navigate foreign code bases ;)

However, I can see people hesitating to adopt this feature, since many repos have one or two extra sub-folders (e.g. "examples", "doc", "repo", etc.). Almost none of the projects hosted at eclipse.org solely rely on those folder names. These repositories would now have some intermediate directories with, and some without pom.xml files, making it more confusing for new contributors.

My bottom line is, that a way to configure "intermediate directories" would be desirable, but I am very grateful for this first step!
Comment 16 Christoph Laeubrich CLA 2019-08-27 05:29:38 EDT
(In reply to Max Bureck from comment #15)
> I think a static configuration would be sufficient for almost all repos. But
> you are right: the limitations you mentioned are against the expectations
> how maven works and would probably surprise users, even if these
> restrictions were documented.
> 
> One could side-step the issue by allowing the configuration only in an own
> config file, e.g. in the reactor's root .mvn directory. On the other hand,
> another custom config file seems like a hollow victory, since we want fewer
> config files and not another one.

I think there was a maven proposal to allow configuration for extensions.xml files but I can't found it right now.

> At least this change produces "soft pressure" to use naming conventions
> making it easier to navigate foreign code bases ;)

As always with convention-over-configuration you are hurt by not follow the convention :-D

> However, I can see people hesitating to adopt this feature, since many repos
> have one or two extra sub-folders (e.g. "examples", "doc", "repo", etc.).

Its still possible to use custom folders in two (and soon three) ways:
- add (or keep) a "connector pom" in the custom folder
- add a tycho.pom listing all subdirectories in the custom folder
- (with Bug 478704 you can add a property to build.properties tycho.pomless.parent=../.. to look at the parents parent folder or any other folder for paren)

Anyways, we can support custom folders if there are enough users. I just don't want to make it too broad so it does not accidental picks ups folders and produces confusion.

> Almost none of the projects hosted at eclipse.org solely rely on those
> folder names.

Theres a world outside eclipse ;-)
I just often has seen this structure on eclipse projects so maybe it is in broader use as thought? Or maybe I just understand it wrong :-)

> have one or two extra sub-folders (e.g. "examples", "doc", "repo", etc.).

I won't mind to add examples/doc/repo to the default mix if there is a demand for it.
Comment 18 Mickael Istria CLA 2019-08-27 05:49:29 EDT
Thanks Christoph for the patch.
I think we can assume this is enough to fix this issue as a lot of customization options are already available. Further enhancement requests should now be tracked in different ticket.
@Christoph: please add a note to the release notes and we can then mark it as resolved.
Comment 19 Christoph Laeubrich CLA 2019-08-27 07:09:43 EDT
Added a note to https://wiki.eclipse.org/Tycho/Release_Notes/1.5#Pomless_Build