AnarchyTools / atbuild

The Anarchy Tools build tool
Apache License 2.0
6 stars 1 forks source link

Add plugin support #84

Closed drewcrawford closed 8 years ago

drewcrawford commented 8 years ago

We're supposed to be building simple, hackable tools but atbuild is becoming more of a monolithic tool. This patch aims to change that.

There are many reasons some feature should not be included in core:

  1. Core needs to be minimal so we can bootstrap it on new platforms; every new feature is a new burden
  2. Core needs to be x-platform but many features do not (see: xcode-emit, packageframework)
  3. Features may want different release frequency (see: xcode-emit) or don't want to coordinate with atbuild
  4. Features may be "not part of AnarchyTools" (Caroline) but still commonly used together
  5. I'm annoyed at upstream over the static linking issue, and thinking of ways to keep us from being that point of central failure that annoys somebody else someday

Therefore, we introduce the world's simplest plugin system designed to move code out of core, or keep code out of core that doesn't need to be.

xcode-emit and Caroline will consume this API. packageframework is a good candidate for a plugin that might be moved out from core.

Documentation to follow

owensd commented 8 years ago

Ok, I get what you're doing now. That's not what I would consider a "plug-in". I consider a plug-in to be a lib that we would load into our binary that implements the Tool protocol.

Also, does something make the Shell command insufficient for what you're doing here? They seem to be very similar.

drewcrawford commented 8 years ago

Also, does something make the Shell command insufficient for what you're doing here? They seem to be very similar.

Did you read the associated doc PR?

Comparison with shell Our shell tool can be used to run any program, not just ones that understand --key value syntax. With plugins, you can write a program (or a wrapper for an existing program) that is specifically designed for Anarchy Tools.

In case you already did, to be more explicit about it:

  1. So far we have one tool (xcode-emit) which could benefit from the key-value clojure syntax enjoyed by other tools but cannot achieve it with shell. Neither IMO is it appropriate for merging with atbuild.
  2. packageframework is basically the opposite; it's in atbuild tree because it wants that first-party clojure syntax, and it was not so large as xcode-emit so the downsides were not as great (but that's not a good excuse). In particular, it doesn't require source-level integration with atbuild (or in-process integration) and it would prefer not to even be compiled for Linux, but that is the price we pay for tight integration right now
  3. Caroline will not be part of AT but similarly would benefit from first-party clojure syntax and desires to integrate tightly with AT (among others)

One's an incident, two's a coincidence, three's a trend.

I consider a plug-in to be a lib that we would load into our binary that implements the Tool protocol.

Let's separate out the naming from the underlying concept just to discuss this. Let's call your thing "in-process plugin" and my thing "out-of-process plugin".

An in-process plugin is useful in the intersection of these circumstances:

  1. Need to manipulate unexposed atbuild datastructures
  2. Don't want to be distributed in atbuild for some reason

I can imagine some set of circumstances where this might be called for but it seems remote to me. At present Swift has no stable ABI, so you'd have to track our snapshot usage really carefully, and it just seems easier to get your changes merged into our tree to solve your problem, unless we are stubborn and won't take them. It could arise later on, and if it does happen let's do an in-process plugin API.

However the problem solved by out-of-process plugins actually has arisen, three times. So that's what I think we should implement, not necessarily to the exclusion of an in-process system someday (after all, we're not opinionated), but in terms of where the demand is at present. At present, there is demand to design external executables around an AT specification, there is not yet anyone trying to get us to dlopen their dylib.

There is also a third thing I did not explore very thoroughly: extending the shell tool itself to meet this need. The problem with that is that if we allow arbitrary key/value in shell than we won't be able to add actual configuration options to shell down the road, which seems limiting. I also think this option, rather than adding ".plugin" to the tool name, more strongly relegates third-party tools to third-class citizens.

In terms of naming, we can call them something other than plugins if you have any ideas. I agree the word "plugin" suggests in-process which is misleading, but I could not think of a word that suggests "written to AT specification but outside of process" so I picked a word that meant only "written to AT specification".

owensd commented 8 years ago

Here's why I don't like the premise of the structure:

  1. It assumes that unordered key/value should be the design for a "plugin". It may be the case that all of your use cases so far meet that, but it's certainly not true of all tools. Writing a wrapper to support/fix the ordering seems counter productive.
  2. It doesn't support the non-value case, such as flags.
  3. It makes that the statement that all "plugins" must have long-names for their key names; a short name for a key name is not allowed.
  4. It makes existing command line tools third-class citizens over a wrapper for that command line tool, and I don't see why or what the value is.

Just noodling about it, I can see a few potential options for expanding the shell support to be richer that would enable both workflows. For example, why can't we simply have an arguments array that provides your key/value pairs?

Anyhow, I've only thought about this briefly in the context of the PR. I'm not suggesting that PR not get merged back to master, but I think we might want to merge shell tools and these out-of-proc plugin tools eventually; they really do seem quite similar to me. I agree there are some differences, I just don't know how significant those differences are right now.

drewcrawford commented 8 years ago

It assumes that unordered key/value should be the design for a "plugin". It may be the case that all of your use cases so far meet that, but it's certainly not true of all tools.

I agree with the desire to support "richer" ways of interacting with a plugin than only key/value. The problem is what that looks like.

One of the things I'm sure it isn't is requiring tools to take an atpkg dependency. atpkg actively burdens something like packageframework, who must take a dependency on all of atpkg in order to read one or two keys that could be more easily exposed another way. A sufficiently complicated tool may legitimately want to take a dependency on atpkg, I have nothing against that; I just have something against forcing it into projects which do not want it, which is the situation now.

That leaves e.g. command-line arguments or environment variables as choices for the interface (e.g., for those tools that do not parse by themselves). --key value seems natural and covers most of the initial usecases so that was what I went with, but if there is some proposal to do something else that's better, I would follow along. Environment variables are the obvious candidate, or inventing a DSL for stdin, those both seem worse to me. atpkg parsing is obviously superior for tools that want to take the dependency, but it does not work for tools that are trying to avoid it.

It doesn't support the non-value case, such as flags.

I would support introducing a syntax for this, maybe something like translating :flag true to --flag.

I currently do not need it, so I did not write it, but that's no vote against it, it is an obvious extension.

It makes that the statement that all "plugins" must have long-names for their key names; a short name for a key name is not allowed.

I don't understand this at all, e.g.

:task {
    :tool "foo.plugin"
    :a "b"
}

here a is a short key because it is a single character.

It makes existing command line tools third-class citizens over a wrapper for that command line tool, and I don't see why or what the value is.

I don't especially envision writing wrappers for underlying command-line programs "just because". I mean, I'm not going to stop you, if you want to build and maintain and update a wrapper for tar, but I agree that might be dumb.

What I am talking about are programs that are clearly designed around a build system to start with. Things that are clearly tools that may be interesting in core, but for the fact that we must keep core small / licensing issues / release schedule / implementation language / doesn't work on linux / etc. This is not for programs you have lying around on your system, but legitimate tools, that just don't live in our tree for "some reason".

For example, you may have used e.g. carthage copy-frameworks where the one and only time you run that program is inside an Xcode Build Script Phase. And if you ever try to run that program elsewhere you will discover it expects Xcode to have communicated some settings to it, it expects to be able to parse an xcodeproj lying around. Its sole purpose is to work around an Xcode bug that complicates Xcode integration with Carthage. It's not a program in its own right.

xcode-emit is very similar: you can't "just" xcode-emit "some files" without knowing how they link together, and their dependencies, and once you've done that you have reinvented atpkg. So xcode-emit has to be designed around a build system. It could be designed around more than one (e.g. if upstream wants to use it). But it's not it's own thing, it is tightly wound to a build system, it has to be, and that is its happy state.

Caroline and packageframework are very similar in that they are new programs, that want to be tightly wound to (at least one) build system. That is the target market for this feature, not tar.

I think we might want to merge shell tools and these out-of-proc plugin tools eventually; they really do seem quite similar to me.

I suspect that some programs prefer to be tightly wound to a build system and other ones do not, and we won't be able to adequately support both usecases with the same solution. Or that if we try to do that, some sad compromise will be reluctantly assented to by everybody.

In many ways that is the current situation: Caroline would like key-value syntax but currently settles for shell. packageframework would like to be distributed separately but currently settles for being in-tree.

I would like to be proven wrong and that we can come up with something that works for everybody, but I think it would take a proposal that works well for at least the three tools in question, without features that negatively impacts shell. Do you know of a such a solution?

drewcrawford commented 8 years ago

I should probably add a key assumption here, for the record.

I believe upstream's biggest mistake so far is to not allow build scripts as part of the build. That seems absurd to me, and I think it's absurd to many people I've overheard complaining about it.

Now having used our script tool for several months: it is very necessary, but I begin to doubt whether it is sufficient. I think we need a second mode for programs written "for" AT, and this is my attempt at what it should be.

You may be right that we need even yet a third mode, to handle the case where the integration is so close it needs to be in-process.

My point is that it is better to have too many ways to integrate with external tools, than too few. Upstream has a lock on the "too few" market. They also have a lock on the "one way to do it" market.

If there is something obvious we can do to shell that makes xcode-emit, packageframework and Caroline happy, I am all for that. Less is more.

However if there is not, I think we should err on the side of interface proliferation. Nobody chooses AT because it is preinstalled, they choose AT because the preinstalled solution is too inflexible to build their software. If we're going to err, I vote to err on the side of being "too" flexible.

owensd commented 8 years ago

It makes that the statement that all "plugins" must have long-names for their key names; a short name for a key name is not allowed. I don't understand this at all, e.g.

:task {
    :tool "foo.plugin"
    :a "b"
}

here a is a short key because it is a single character.

Here's the code:

cmd += "--\(key) \"\(evaluateSubstitutions(input: value, package: task.package))\" "

So regardless of the length of the key, it's always prefixed with -- which does not properly parse in all CLI tools. Exampe: ls --l vs. ls -l.

So you'd need to fix that.

It assumes that unordered key/value should be the design for a "plugin". It may be the case that all of your use cases so far meet that, but it's certainly not true of all tools. I agree with the desire to support "richer" ways of interacting with a plugin than only key/value. The problem is what that looks like.

I'm not talking about needing special code to handle the parameters as in a dynamically loaded plugin, I'm simply talking about command line tools that have order dependent arguments.

Example: find . -name "*.swift" -print vs. find -print . -name "*.swift"

One of these usages is valid, the other is not.

You may be right that we need even yet a third mode, to handle the case where the integration is so close it needs to be in-process.

My point is that it is better to have too many ways to integrate with external tools, than too few. Upstream has a lock on the "too few" market. They also have a lock on the "one way to do it" market.

I agree. I think it should be easy to integrate with existing tools with as little fuss as possible. I also agree that that is not what this particular PR is addressing, so let's table that discussion until we really have a need for it.

What I am saying is that I think we can merge the Shell and this proposal together as a single: "hey, this is how to run external tools.". I agree that this will take some more thought and brainstorming. I agree that your solution works for a specific set of tools.

My Recommendation: Rename this from "plugin" to maybe something like "external tool", log a bug to consider merging Shell and this, and merge it in.

drewcrawford commented 8 years ago

I have failed to explain something important.

If you think about what the word "plugin" connotes (which has problems as a word, but not this problem): it means something that is written specifically for the host. As an implementation detail, I happen to have designed this to use command-line programs, but that does not make every command-line program a sensible plugin any more than under your in-process scheme libopenssl.dylib is a sensible in-process plugin.

So regardless of the length of the key, it's always prefixed with -- which does not properly parse in all CLI tools. Exampe: ls --l vs. ls -l.

Yes, but ls is not written to our specification. So you should call it with shell.

I'm not talking about needing special code to handle the parameters as in a dynamically loaded plugin, I'm simply talking about command line tools that have order dependent arguments.

Example: find . -name "*.swift" -print vs. find -print . -name "*.swift"

Yes, but find is not written to our specification. So you should call it with shell.

You have identified exactly the circumstances that motivated the design of a plugin interface to begin with. CLI is messy, and there are twenty ways to parse arguments. While that is all fine and good for UNIX, and for shell, that flexibility prevents us from using shell to design a "proper" third-party tool.

Think about what it means to be a "proper tool", e.g. consider atllbuild:

I want to write "this sort of thing" that doesn't necessarily live in the core tree. My goal is not to introduce a new and incompatible way to call find; shell is good enough. But shell is not good enough to implement a second atllbuild. shell is not good enough to implement a Swift preprocessor. shell is not good enough to build an IPA. These are the kinds of things I am trying to do. If I have only invented a second way to call find then I have failed.

I think we can merge the Shell and this proposal together as a single: "hey, this is how to run external tools.".

On the other hand if this interface happens to lead to an second way to call find, that's okay. I don't especially think we need one, but this is Anarchy Tools. Do everything seven ways.

I just don't see a solution to, for example, the "unordered keys" problem that does not make one interface worse. As you say, find cares about argument order. atllbuild does not. I do not know of any way to resolve that problem within one interface.

Rename this from "plugin" to maybe something like "external tool", log a bug to consider merging Shell and this, and merge it in.

.externaltool is not terrible but it does permit the misunderstanding that the feature is designed for the likes of find (both "external" and a "unix tool"), as opposed to programs that meet our specification. What about something like .customtool or .attool? Neither of those include find, and perhaps they avoid the drawbacks of the word "plugin" we identified earlier.

owensd commented 8 years ago

I have failed to explain something important.

  • This feature is for programs that are written specifically to our specification.
  • Shell is for programs that are not written specifically to our specification.

Nope, we're on the same page. The only point I'm trying to make is that I think there may be a way to removal that distinction in the future, but maybe I'm wrong on that.

The only part that I'm really take issue with is "plug-in". I can very much see the time when I want to build a plug-in that's not a tool, but just a library that we load and invoke (like our other tools). Maybe we call those custom tools and what you have plug-ins though.

In any respect, I think your intention is quite clear. I don't have any reservations against it. Let's just have the docs for it be a little more clear; I think a lot of what is in your last post could be added to the docs and they'd be golden.

In other words, stop talking already and check it in! ;) 👍