switch plugins to use AutoService

[arifogel]

This change is 

[dhalperi]

I'd probably wrap this in a try/catch and report an error loading any of these but continue loading others?

[arifogel]

Hmm I don't know. I don't think it really makes sense to keep running when a plugin has failed. How about catch, wrap, and throw?
EDIT: Or alternatively, record, load remaining, and throw CompositeBatfishException?

[arifogel]

Addressed via catching of ServiceConfigurationError, throwing CompositeBatfishException after all are done loading.

[arifogel]

Rebased and resolved conflict with OutliersQuestionPlugin.java (added AutoService annotation, imports on top of master version)

[dhalperi]

Reviewed 70 of 71 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

projects/batfish/batfish, line 5 at r2 (raw file):

BATFISH_PATH="$(dirname $BATFISH)"
ALLINONE_PATH="${BATFISH_PATH}/../allinone"
ALLINONE_JAR="$ALLINONE_PATH/target/allinone-bundle-${BATFISH_VERSION}.jar"

What's the process you use to decide to include ${X} vs just $X?

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/plugin/Plugin.java, line 8 at r2 (raw file):

  @Override
  public final int compareTo(Plugin o) {

Should this really be final? Would plugin equality not need to be tested later for extensions that care about other fields?

---

Comments from Reviewable

[arifogel]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

projects/batfish/batfish, line 5 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

What's the process you use to decide to include ${X} vs just $X?

Really it is only necessary when there is a separator character (e.g. "-") in the name. But I like to use it all the time. Sometimes I am weak and forget.

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/plugin/Plugin.java, line 8 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Should this really be final? Would plugin equality not need to be tested later for extensions that care about other fields?

The hope is no. Can you provide a plausible counterexample?

---

Comments from Reviewable

[dhalperi]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/plugin/Plugin.java, line 8 at r2 (raw file):

Previously, arifogel (Ari Fogel) wrote…

The hope is no. Can you provide a plausible counterexample?

Perhaps a better question would be why these need to be comparable at all; what's this actually used for?

If it's for putting them in some sorted order, probably makes more sense to have a delegate comparator of the names than to actually make Plugin itself comparable.

---

Comments from Reviewable

[arifogel]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/plugin/Plugin.java, line 8 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Perhaps a better question would be why these need to be comparable at all; what's this actually used for?

If it's for putting them in some sorted order, probably makes more sense to have a delegate comparator of the names than to actually make Plugin itself comparable.

Yes it is just for sorting. From my perspective, if it is ok for equals and hashCode being defined as they are and final, then it is not a jump to accept compareTo existing as implemented and being final. If someone wants to sort in a different order or using different fields, they can use a different comparator.

---

Comments from Reviewable