librelist archives

« back to archive

Project merging based on keys

Project merging based on keys

From:
Jean Niklas L'orange
Date:
2013-01-05 @ 13:10
Hello,

I've felt that the project merging part of Leiningen turns more and more
into spaghetti-like code, and it looks like it's smart to find a better way
of merging maps. The current issues with merging—from what I can see—is the
following:

   - It is currently messy to add specific merging based on keys: For those
   unaware, Leiningen is using the :reduce metadata flag on an initial
   value to e.g. merge repositories and dependencies correctly. As it for me
   seems like a lot of different keys need special handling, making this easy
   and not as hackish sounds important.
   - For plugins, adding an initial value inside Leiningen's *project.clj* with
   a ^:reduce function is impossible (or an incredible hack). As such,
   plugins cannot add keys which need special handling by them, or extend the
   already existing ones.
   - It makes sense to let the specific merge functions be associated with
   keys, not the values associated to the keys: You would relate
   :hours-worked with addition and :probability with multiplication, even
   though both works on numbers.

So I think a good refactor of the merge stuff should solve these problems
by:

   - Associate specific merging with keys, and default to "standard"
   merging if no specific handling is needed.
   - Make it easy to add new specific merge handlers, preferably without
   adding it all together within a large conditional block.
   - Make it possible for plugins to "hook in" new specific handling if
   needed.

See #878 <https://github.com/technomancy/leiningen/issues/878> for the
specific merge handling issue.

Is this indeed an issue, or have I been overthinking these problems, and
has anyone else except me thought about how to solve this?

-- 
Regards,
Jean Niklas L'orange

Re: [leiningen] Project merging based on keys

From:
Justin Balthrop
Date:
2013-01-05 @ 15:30
Hi Jean,

I spent some time refactoring this a while back, and the result was the 
addition of the :reduce behavior you describe.

I considered adding a multimethod so that merging could be done based on 
the key, but decided this was probably overkill without further use cases 
that required it. This may in fact be the right way to go now, but I'd 
need some more concrete examples to be convinced.

As far as plugins adding initial values in project.clj, isn't this what 
project middleware is for? Adding a some middleware to accomplish this 
doesn't seem like an incredible hack to me. Or am I missing something?

Best,
Justin



On Jan 5, 2013, at 5:10 AM, "Jean Niklas L'orange" <jeannikl@hypirion.com> wrote:

> Hello,
> 
> I've felt that the project merging part of Leiningen turns more and more
into spaghetti-like code, and it looks like it's smart to find a better 
way of merging maps. The current issues with merging—from what I can 
see—is the following:
> It is currently messy to add specific merging based on keys: For those 
unaware, Leiningen is using the :reduce metadata flag on an initial value 
to e.g. merge repositories and dependencies correctly. As it for me seems 
like a lot of different keys need special handling, making this easy and 
not as hackish sounds important.
> For plugins, adding an initial value inside Leiningen's project.clj with
a ^:reduce function is impossible (or an incredible hack). As such, 
plugins cannot add keys which need special handling by them, or extend the
already existing ones.
> It makes sense to let the specific merge functions be associated with 
keys, not the values associated to the keys: You would relate 
:hours-worked with addition and :probability with multiplication, even 
though both works on numbers.
> So I think a good refactor of the merge stuff should solve these problems by:
> Associate specific merging with keys, and default to "standard" merging 
if no specific handling is needed.
> Make it easy to add new specific merge handlers, preferably without 
adding it all together within a large conditional block.
> Make it possible for plugins to "hook in" new specific handling if needed.
> See #878 for the specific merge handling issue.
> 
> Is this indeed an issue, or have I been overthinking these problems, and
has anyone else except me thought about how to solve this?
> 
> -- 
> Regards,
> Jean Niklas L'orange

Re: [leiningen] Project merging based on keys

From:
Jean Niklas L'orange
Date:
2013-01-06 @ 00:12
Hey Justin,

On 5 January 2013 16:30, Justin Balthrop <justin@justinbalthrop.com> wrote:

> I considered adding a multimethod so that merging could be done based on
> the key, but decided this was probably overkill without further use cases
> that required it. This may in fact be the right way to go now, but I'd need
> some more concrete examples to be convinced.
>

Well, it mainly depends on how much functionality we should have and how
much logic we should add into this. Granted, here is an assumption that
these will clash, but of course this rarely happens. Here are some:

   - [:repl-options :init] should be added through (list 'do left right) -
   concatenating may give errors (#878)
   - [:repl-options :prompt] should pick rightmost value, instead of
   concatenating fns, same for [:repl-options :nrepl-handler] (and possibly
   :nrepl-middleware ?)  - otherwise it may spit errors
   - [:repl-options :timeout] should pick maximal value, not rightmost
   - [:aliases alias-name] may concatenate vectors, or throw a type
   mismatch if one is a string and another is a vector. Should pick leftmost
   instead and issue a warning, because it may spit out errors when vectors
   are concatenated.
   - [:min-lein-version] may pick a lower minimum version because it picks
   the rightmost value, thereby resulting in unintended lein behaviour
   - [:dependencies], [:repositiories] and friends (deploy-repos and
   plugin-repos as well), but these are already covered
   - [:javac-options] and [:jvm-opts] may contain multiple arguments when
   concatenated (or maybe other parts should handle multiple similar flags on
   path?)
   - [:warn-on-reflection] should possibly pick their logical OR (true if
   one of them is)
   - [:pom-additions] currently spits out erroneous XML when the sexps are
   concatenated
   - [:aot] may discard an :all option in favour of a vector, and will
   compile twice if an element is in the list twice

some others such as [:mailing-list] and [:license] should spit out an error
when a conflict occurs, instead of recursively merging their contents.

As I mentioned, it is seldom the case these keys are merged, at least for
the majority of them. As such, it may be overkill to introduce correct
behaviour for each of them, even though they certainly spit out
erroneous/unsatisfactory output. (Though I'd think it would be a
nice-to-have if it's easy to put in and doesn't add too much complexity)

As far as plugins adding initial values in project.clj, isn't this what
> project middleware is for? Adding a some middleware to accomplish this
> doesn't seem like an incredible hack to me. Or am I missing something?
>

Right, I forgot about middleware. Surely, that would work to add in new
initial values and new ^:reduce functions. Some cases may not be able to
use a reduce function though, e.g. when picking one out of two ints or two
strings. And while it seem very unrealistic that two plugins use the same
initial value, only the latest middleware applied's :reduce function would
be used.

I'm just pointing out possible complexity, and wonder if it's of interest
to look for solutions to it.

-- 
Regards,
Jean Niklas L'orange