Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/warbler/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ class Config
attr_accessor :features

# Traits: an array of trait classes corresponding to
# characteristics of the project that are either auto-detected or
# configured.
attr_accessor :traits
# characteristics of the project that are either auto-detected or
# forced enabled during `Config.new(forced_traits: [...]) do |config|`
attr_reader :traits
Copy link
Copy Markdown
Contributor

@chadlwilson chadlwilson Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why this attribute is also in the Traits super? Do we need to change both? (i'm not an expert with this particular Ruby magic)

attr_accessor :traits

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually removed that other one in this PR, since the Traits module never uses it. Config includes Trait, and having both (previously attr_accessor :traits) doesn't do any harm, but is redundant.

I changed it to attr_reader so you cannot assign a new array of Traits in Config, but actually it doesn't prevent you from manipulating the existing array. This is a way that one could remove an unwanted Trait (config.traits.reject! { |t| t.is_a?(Warbler::Traits::Gemspec) }) but doing so does not remove the config changes that that Trait will already have made in before_configure. (I'm just thinking out loud here).


# Directory where the war file will be written. Can be used to direct
# Warbler to place your war file directly in your application server's
Expand Down Expand Up @@ -183,8 +183,8 @@ class Config
attr_reader :warbler_templates
attr_reader :warbler_scripts

def initialize(warbler_home = WARBLER_HOME)
super()
def initialize(warbler_home = WARBLER_HOME, forced_traits: [])
super(forced_traits)

@warbler_home = warbler_home
@warbler_templates = "#{WARBLER_HOME}/lib/warbler/templates"
Expand Down
26 changes: 22 additions & 4 deletions lib/warbler/traits.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,28 @@ module Warbler
# the kind of project and how it should be packed into the jar or
# war file.
module Traits
attr_accessor :traits

def initialize
def initialize(forced_traits = [])
@forced_traits = forced_traits
@traits = auto_detect_traits
Comment on lines +19 to 20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of a nitpick, but perhaps we don't need to store the forced_traits and can just pass them to auto_detect_traits for consideration, and continue to store only the "resolved" traits. Avoids the coupling ion the instance vars between calls and makes it a bit more obvious that auto_detect_traits is not solely auto detection - without have to rename the method and potentially breaking any overrides/monkey-patches.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did both during development 🤣. In the end I decided to assign it to an instance var in case it became useful to know what was forced later. I'm not married to it.

end

def auto_detect_traits
TraitsDependencyArray.new(Traits.constants.map {|t| Traits.const_get(t)}).tsort.select {|tc| tc.detect? }
all_trait_classes = Traits.constants.map { |t| Traits.const_get(t) }
sorted = TraitsDependencyArray.new(all_trait_classes).tsort

conflicts = @forced_traits.flat_map { |ft| ft.conflicts }.uniq

sorted.select do |tc|
if @forced_traits.include?(tc)
true
elsif conflicts.include?(tc)
false
elsif tc.requirements.any? && tc.requirements.all? { |req| conflicts.include?(req) }
false
else
tc.detect?
end
end
end

def before_configure
Expand Down Expand Up @@ -53,6 +67,10 @@ module ClassMethods
def requirements
[]
end

def conflicts
[]
end
end

def self.included(base)
Expand Down
4 changes: 4 additions & 0 deletions lib/warbler/traits/jar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ def self.detect?
!War.detect?
end

def self.conflicts
[War]
end

def before_configure
config.gem_path = '/'
config.pathmaps = default_pathmaps
Expand Down
6 changes: 5 additions & 1 deletion lib/warbler/traits/war.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ class War
DEFAULT_GEM_PATH = '/WEB-INF/gems'

def self.detect?
Traits::Rails.detect? || Traits::Rack.detect?
Rails.detect? || Rack.detect?
end

def self.conflicts
[Jar]
end

def before_configure
Expand Down
58 changes: 58 additions & 0 deletions spec/warbler/traits_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,62 @@
end
end
end

describe "conflicts" do
it "Jar conflicts with War" do
expect(Warbler::Traits::Jar.conflicts).to include(Warbler::Traits::War)
end

it "War conflicts with Jar" do
expect(Warbler::Traits::War.conflicts).to include(Warbler::Traits::Jar)
end

it "traits with no declared conflicts return an empty array" do
expect(Warbler::Traits::Bundler.conflicts).to eq([])
end
end

describe "forced_traits" do
context "in a Rack project with Bundler" do
run_in_directory 'spec/sample_bundler'

it "auto-detects War trait by default" do
config = Warbler::Config.new
expect(config.traits).to include(Warbler::Traits::War)
expect(config.traits).to include(Warbler::Traits::Rack)
expect(config.traits).to_not include(Warbler::Traits::Jar)
end

it "forces Jar and excludes War when Jar is forced" do
config = Warbler::Config.new(forced_traits: [Warbler::Traits::Jar])
expect(config.traits).to include(Warbler::Traits::Jar)
expect(config.traits).to_not include(Warbler::Traits::War)
end

it "excludes traits that require an excluded trait" do
config = Warbler::Config.new(forced_traits: [Warbler::Traits::Jar])
expect(config.traits).to_not include(Warbler::Traits::Rack)
end

it "preserves non-conflicting auto-detected traits" do
config = Warbler::Config.new(forced_traits: [Warbler::Traits::Jar])
expect(config.traits).to include(Warbler::Traits::Bundler)
end

it "runs before_configure with forced traits" do
config = Warbler::Config.new(forced_traits: [Warbler::Traits::Jar])
expect(config.jar_extension).to eq('jar')
end
end

context "with no forced traits" do
run_in_directory 'spec/sample_jar'

it "behaves identically to auto-detection" do
default_config = Warbler::Config.new
forced_config = Warbler::Config.new(forced_traits: [])
expect(forced_config.traits).to eq(default_config.traits)
end
end
end
end