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

Use of :paths outside a project is not recommended #66

Closed
puredanger opened this issue Jan 19, 2021 · 11 comments
Closed

Use of :paths outside a project is not recommended #66

puredanger opened this issue Jan 19, 2021 · 11 comments
Assignees
Labels
core Related to core functionality of Polylith

Comments

@puredanger
Copy link

Describe the bug
The polylith directory structure instructs users to create projects with a deps.edn with :paths outside the project itself:

:paths ["../../components/user/src"
         "../../components/user/resources"
         "../../bases/cli/src"
         "../../bases/cli/resources"]

This is generally a bad idea. The intent of :paths is to describe paths in your own project. In the polylith setup, these are effectively local dependencies. In the future, use of paths that are not under the project root may emit a warning or even an error.

deps.edn supports local dependencies using the :local/root coordinate attribute. Relative paths are supported there.

:deps {my.component/user {:local/root "../../components/user"}
       my.base/cli {:local/root "../../bases/cli"}

You would also need deps.edn files under ../../components/user with :paths ["src" "resources"] etc of course.

I can't fully assess what other impacts this may have on the polylith architecture, but using local deps is strongly preferred over referring to non-project paths. If for some reason this is not sufficient, I'd appreciate talking through why (I know there are some issues others have raised for monorepo style setups).

Alternately, using a deps.edn at the root of the entire project may also be an option, if you want to consider the entire thing as a single project.

@furkan3ayraktar
Copy link
Collaborator

Hi Alex! Thank you for your interest in Polylith and creating this issue to help us make it even better.

I’ll begin my response by quickly explaining the relevant parts of the high-level design of Polylith, and how that affects the various approaches we tried with deps.edn, before landing on using the relative paths.

Projects in a Polylith workspace are the way to configure deployable artifacts. Each project can be deployed as a jar file or as a Datomic Ions revision, etc. So the projects deps.edn is just a way to pick a set of components, a base, and external dependencies to create an artifact. Additionally, there is a :test alias in the deps.edn where the directories containing tests for each component are included.

Components are encapsulated blocks of code, which can be assembled together in projects with other components, a base, and external dependencies. We do not see them as local libraries or dependencies, rather we see them as a living part of a single codebase.

So that’s a quick recap of relevant parts of the documentation, and now I can explain what we’ve tried so far.

First, instead of having one deps.edn file per project under the projects directory, we tried having an alias per project in the root deps.edn file. Even though that risked it becoming a huge deps.edn file, we thought it might be a better solution than many different files. However, we hit a big issue when I tried it on one of my Datomic Ions projects; where the Ions commands did not respect the aliases provided. Datomic Ions commands assumed that the sources are listed under the :paths key and ignored the :extra-paths provided in the aliases, which I ran the command with. When we saw this we thought maybe similar issues will happen with other tools. Also, when we came up with the final idea we thought it’s simpler and cleaner to keep each project’s deps.edn under its own directory, rather than one big deps.edn.

We also tried using the :local/root approach. There we had two issues: 1) adding a deps.edn file per component is too much unnecessary boilerplate, and adds friction to the development experience, 2) we can’t refer to aliases in the component’s deps.edn via :local/root. The second point here is important since the test directories are listed under a :test alias and we can’t reference a specific alias of a deps.edn project via this :local/root approach. This makes it impossible to run tests for a specific project referring to components via :local/root.

Using relative paths in the :paths section of deps.edn solves both the problems from the other two approaches, and most importantly it solves them without adding any extra complexity. Simplicity and productivity are the two guiding design principles in Polylith, so we’d prefer not to compromise with a solution that negatively affects either of them.

We would love to have a chat more about this with you and figure out how we can solve this problem, without introducing any new complexity or friction to the users of Polylith.

Cheers,
Furkan

@furkan3ayraktar furkan3ayraktar self-assigned this Jan 20, 2021
@furkan3ayraktar furkan3ayraktar added the core Related to core functionality of Polylith label Jan 20, 2021
@puredanger
Copy link
Author

Thanks, that's a helpful overview. Given what you said, I think having a root deps.edn is probably what I would recommend in this case and is the best mapping. With respect to the Ion commands, I think maybe you gave up too quickly there. The tricky thing with the ion tools is that you are running the tool, so classpath-modifying options affect the tool itself, not the libraries being assembled by the tool. I believe the ion dev tools have a way to do this but I will follow up with the Datomic team and get back to you.

@tengstrand
Copy link
Collaborator

In Polylith, the workspace is the root of all our code, which allows us to share code between all our projects, e.g.:

▾ workspace
   ▸ components
   ▸ bases
   ▾ projects
      ▾ project1
           deps.edn
      ▾ project2
           deps.edn
   deps.edn

The development project lives at the root, while the other projects live under the projects directory. This keeps things simple and separated, and allows us to reuse all our building blocks (components and bases) in all our projects by specifying each of them in a single config file, which makes each deps.edn file very cohesive and readable. In a monorepo like this, we consider the workspace to be the root and all projects should be allowed to refer to any directory within that root directory.

This is a very simple and powerful way of organising code, and allows us to separate development from how things are executed in production, while giving us a single development experience with “living code” that we can change and get instant feedback from.

To force people to merge all those project files into one single file would be painful, because they represent different artefacts and should naturally live in separate deps.edn files. Each project can also have their own aliases for different purposes, and to merge all of them into a single deps.edn at the root will both make things messy and make people lose the context. We prefer to keep it simple and we think that the whole Clojure community could benefit a lot if this could be allowed even in the future.

So to be able to give the users of Polylith the best possible experience, we would appreciate it if you would consider supporting the way Polylith currently structures the code.

We understand that you have many other use cases to consider, and that Polylith is not in any kind of position to dictate any of your design decisions. However, we’re hoping that there’s a way to solve this issue, which keeps the simplicity and clarity of Polylith’s current structure intact, and still gives tools.deps the control it needs over the content of the deps.edn files.

BR,
The Polylith Team

@puredanger
Copy link
Author

It seems like you are trying to both have independent reusable pieces of code (what we'd call libraries usually) while also not using any of the tools we use to make code independent and reusable. (In Simple vs Easy terms, this seems very much on the side of "easy" rather than "simple".)

Re:

To force people to merge all those project files into one single file would be painful, because they represent different artefacts and should naturally live in separate deps.edn files.

You've already forced people to merge all of the project files in the deps.edn at the root (and in each project deps.edn). As you say, "they represent different artefacts and should naturally live in separate deps.edn files" - this same reasoning should apply to components and bases too, should it not? The components have no statement of dependencies so they cannot even be understood without some greater root, which I don't like at all.

From the "living code" aspect, you seem to be trying to get away from building and consuming artifacts which is where a lot of the perceived "heaviness" of using multiple projects comes from, and I think that's a good goal and is in fact why we have local deps in deps.edn. They allow you to have these references to source-based projects without needing to make or consume artifacts. You seem resistant to making a deps.edn for those but a starting one for a component without dependencies is just: {:paths ["src" "resources"]} which hardly seems burdensome. Components with deps could then declare that dep in exactly one place instead of the N distant deps.edn's where you declare it now.

The tooling concern for the intent of "run all tests" I agree is not well served without additional assistance right now (I think we will make this better and are tracking this need at https://ask.clojure.org/index.php/9849/teams-common-dependencies-tooling-across-multiple-projects). In the meantime, you could make a polylith tool run by clj -X that understood your polylith project structure and provided whatever common tools were useful.

In summary, I strongly think your components and bases are mini-projects, should have deps.edn files, and should be referred to from your projects as local deps. I think this adds minimal additional work to polylith users and better factors things like dependency definition to reduce duplication. It's also more in line with where clj and deps.edn are going as tools and what ion-dev tools are likely to support.

@furkan3ayraktar
Copy link
Collaborator

Hi Alex! Again, we greatly appreciate your messages and they really made us think outside of the box.

The components in Polylith differ from libraries significantly. A library clearly states the dependencies it needs and it can be bundled and released under a version. However, a Polylith component is part of the whole codebase and what libraries it uses are only specified in the project(s) it is used in. Another important thing to note is that Polylith components only know about interfaces, and they are not aware of what concrete components they depend on. This decision is made only when it’s time to combine a component with other components into one or several projects. In other words, a component can’t live alone, isolated from other components, and this is the way they become isolated, decoupled, and reusable, Lego-like blocks of code. In summary, we still can’t think of components as mini-projects because they lack many of the characteristics that define a project.

At first thought, we also considered having deps.edn for each component and base. However, when we dug deep into the topic, we realized that if we have deps.edn for each component, those will be misleading since they won’t be listing either external or internal dependencies. Of course, we could still have them to just make Polylith work with tools.deps and Clojure CLI, as you suggested, however this would be rather a workaround. Also, this would limit our ability to run incremental tests based on recent changes, since we won’t be able to pick which component tests to run in the project configuration through the :local/root approach.

Another solution would be for us to develop our own Polylith configuration file. Instead of using deps.edn with paths outside of the project directory, we could create another edn file named polylith.edn for each Polylith project. This configuration file could list all the components, base, and dependencies for a particular project. In the Polylith command line tool, we could read these configuration files and generate a classpath using the tools.deps API. This classpath could be then passed to Clojure CLI by using the -Scp option so that any other tool or command can be run using the correct classpath for a specific project. At the root of the codebase, we can still have our root deps.edn which will then only be used by the IDE/editor during development. This way we will not interfere with users' choice of development environment.

Another benefit of this solution could be that the Polylith tool would be independent of build tools and could be used by users of Leiningen or Boot. If the user prefers, they can use Leiningen for their development environment, whereas Polylith tool will be using tools.deps API internally when creating a classpath for projects when it comes to creating artefacts.

I have included a diagram below describing the proposed solution and we would like to hear your thoughts on this.

Polylith - Idea - Idea 2

Cheers,
The Polylith Team

@puredanger
Copy link
Author

Seems like a reasonable solution. You could probably generate the deps.edn too.

I still think it's weird that you have to restate the dependencies of every component in every project, rather than stating those in the component itself once.

@tengstrand
Copy link
Collaborator

Hi again.

Let me try to explain this by using an example.

In the object oriented world, we can depend on an interface without knowing what concrete class it implements. This is how we decouple code and allow other parts of the codebase to decide what concrete class to instantiate, without affecting the user of the interface. If we had to force the user of the code to know the concrete class, then they wouldn’t be decoupled and the polymorphic mechanism wouldn’t work.

Component interfaces use the same idea, that the code that uses an interface shouldn’t know the concrete component behind the interface. If we are forced to specify the concrete components it uses in each component, then it wouldn’t work either, in the same way it didn’t work in the OO example.

Let’s say we have the component interface invoicing, and two components that “implement” that interface, invoicing and invoicing-remote. By letting each project decide what concrete components to use, we can now use the invoicing component in the development project while in one of the production projects we could use invoicing-remote. All code that depends on the invoicing interface wouldn’t care whether it depended on the invoicing or invoicing-remote component.

Because we can’t specify which concrete components to use for each component, it will not work to have a deps.edn file per component with an empty list of components. Instead, this is specified in each project, along with the libraries. So to summarise, a component doesn’t know what components it depends on, and it doesn’t know what version of its libraries it depends on. Those decisions are instead made by each project.

We hope this answered your question and if not, now could be the time to book a short meeting to dispel any ambiguities, because explaining things in text has its limitations.

Cheers,
The Polylith Team

@puredanger
Copy link
Author

What you're saying about interfaces and implementations makes sense but does not seem to match what I see in example projects, so maybe that's why I'm confused. For example, this database component has an interface that has no dependencies but uses java.jdbc and honey in it's implementation. When a project wants to use the database component, I don't understand how you can know to include java.jdbc other than by manually looking through all the components source code and guessing what dependencies you need to include. And you need to do that each time you make a project or change the components a project uses (and for the root deps.edn). If the interface and implementation were actually separate, and the implementation declared what its own deps were, then you could just specify the set of components to use and you would automatically get the right set of deps.

@tengstrand
Copy link
Collaborator

tengstrand commented Jan 27, 2021

You are absolutely right that specifying what libraries a component uses is not a bad idea at all. The good thing with it is that you are explicit about what libraries each component uses, and that was also how we implemented it in the Leiningen version.

In this version we went for a more decoupled approach, you could say. When you start to use a new library in a component, you first add it to the development project, so that you can work with the code from the single development environment. Then you add a row in the :ns-to-lib key in the deps.edn file at the root, e.g. “clojure.java.jdbc org.clojure/java.jdbc”, for the org.clojure/java.jdbc {:mvn/version "0.7.5"} library, where the first key is the top namespace of that library.

When you later create a new project and add the database component to it, the tool will remind you that you also need to add e.g. the org.clojure/java.jdbc library, by checking the component’s :require statements to figure out what namespaces it uses, in combination with the :ns-to-lib mapping. The libs command can then be used to get an overview of what libraries each project, component and base uses.

The problem can be solved in many different ways, but this has worked very well for us, and allows us to focus on the development experience.

@furkan3ayraktar
Copy link
Collaborator

Fixed in #94

@tengstrand
Copy link
Collaborator

Hi Alex!

Today we finally released v0.2.0-alpha10 that includes this issue.

I was just going to say that it has worked really well to let each brick keep track of its own sources and dependencies and that this change has made the tool even better!

/Joakim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core functionality of Polylith
Projects
None yet
Development

No branches or pull requests

3 participants