Homebrew / brew

🍺 The missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
40.22k stars 9.44k forks source link

Add `livecheck` Formula DSL #7027

Closed MikeMcQuaid closed 3 years ago

MikeMcQuaid commented 4 years ago

Homebrew/livecheck provides various automated ways of detecting formulae updates. A livecheck DSL should be added in Homebrew/brew and used by Homebrew/homebrew-core formulae and migrated from Homebrew/livecheck's Livecheckables directory and brew livecheck use this new data.

No need to say "I'm planning on working on this" or similar in this issue but feel free to open a PR that doesn't work (yet!) and ask for help.

nandahkrishna commented 4 years ago

Hi, I'd like to work on this. Any pointers to get started?

MikeMcQuaid commented 4 years ago

@nandahkrishna If you ask any more specific questions we can help!

samford commented 4 years ago

For what it's worth, I have some forthcoming changes to livecheck that I will be proposing relatively soon (basically, allowing for some additional ways of matching and manipulating version strings) and this could impact the design/implementation of the DSL in some ways (it may be minor, though).

This isn't to say that you can't or shouldn't work on this now, by any means, but that you would have to keep track of changes to livecheck and incorporate them into the DSL as they're merged.

nandahkrishna commented 4 years ago

Alright, thanks. I’m still looking through the code to see how it works. I’ll keep track of the changes in livecheck and make changes when required.

MikeMcQuaid commented 4 years ago

@samford Can you detail what you are thinking of in this thread for discussion?

nandahkrishna commented 4 years ago

Hello, I had a look at the code in the livecheck repository and in general, how the brew commands have been written and structured here.

Hope this isn't a silly question, but would our aim here be to create a new livecheck.rb in Library/Homebrew, reimplement the functionality from the livecheck repository and migrate the Livecheckables here?

MikeMcQuaid commented 4 years ago

Hope this isn't a silly question

@nandahkrishna No such thing!

but would our aim here be to create a new livecheck.rb in Library/Homebrew

Perhaps but that would be in Library/Homebrew/dev-cmd and probably be the last thing if we wanted to remove the tap completely. Instead the priority should be:

Make sense?

Moisan commented 4 years ago

@MikeMcQuaid correct me if I'm wrong, but currently Livecheckables are made of a method that returns a hash. I think we would want a block like this instead:

livecheck do
  url "https://aacgain.altosdesign.com/alvarez/"
  regex /aacgain-(\d+(?:\.\d+)+)\.tar/
end
MikeMcQuaid commented 4 years ago

@Moisan Something like that, yeh. Could you detail in here all the different fields/data you see being useful?

nandahkrishna commented 4 years ago

@MikeMcQuaid yep that does make sense. I'll look through formula.rb and think it over then. Thanks!

Moisan commented 4 years ago

There is currently no other inputs to livecheck outside of the information available in the Formula and an optional Livecheckable. However we could probably help the livecheck heuristic a bit by pointing on the right format. For example something like :using => :gnome could force to use gnome versioning (which skips odd versions since they are not stable). So far this is done by matching the url on different regex to find the right versioning.

There is also some information hardcoded in livecheck heuristic.rb, such as the formulae to not check with Github versioning even if their url includes github. The equivalent in a formula would be something like :not_using => :github. Same goes for some sourceforge formulae.

A tag such as allow_pre_release could be used in some very specific case. It goes against Homebrew policy to include pre release code but some software seems to forever be in pre-release (e.g., black). Some recent changes now remove pre-releases from livecheck results.

This is all the details I can think of for now.

MikeMcQuaid commented 4 years ago

Thanks @Moisan, that's really helpful.

samford commented 4 years ago

Can you detail what you are thinking of in this thread for discussion?

Thierry already mentioned the idea of adding the ability to specify how livecheck's heuristic should approach a formula (rather than guessing), so +1 on that.

Allow for text to be replaced in version strings. For example, the version in a formula might be 1.2.3 but the version from livecheck may be 1_2_3 and this would always lead to a mismatch. If we could provide regex/string replacements in the livecheckable, this kind of situation could be addressed.

Create an assortment of built-in version-matching regexes that can be referenced in livecheckables, so we don't have to copy-paste the same regex code over and over (with small changes, if any).

Add prefix and suffix regex options, to be prepended/appended to the main regex within livecheck. This is somewhat related to the built-in regexes idea, as it would allow us to make livecheckable's that are a bit more DRY. You could specify the built-in regex to use (or potentially lean on the heuristic) and only have to provide a specific prefix or suffix for matching. For example, a git repo may contain tags for various projects but we only want to match tags starting with projectname-, followed by the version.

Add the ability to completely skip livecheck for a given formula. Some formulae are important but are unmaintained and won't receive further updates (e.g., imap-uw). In this case, we want to keep the formula around but we don't need livecheck to waste time/effort only to give an error. This could be something like, :skip => "Not maintained", where the optional text could explain why the formula is being skipped.

Also +1 on the idea of allowing pre-release versions. How to best approach this (and whether we would need an explicit allow_pre_release tag) will likely be affected by the above ideas.


I've tried to keep things relatively brief here, so that this issue doesn't get derailed. If any of these seem like valuable additions, I can create issues in the homebrew-livecheck repo to discuss them further (e.g., interactions between these ideas, design/implementation, etc.). I'm still thinking about these ideas and experimenting with how they could be implemented, which is why I haven't already created issues.

I think this is everything I've come up so far that would affect the design/implementation of the livecheck DSL.

MikeMcQuaid commented 4 years ago

Allow for text to be replaced in version strings.

👍

Thoughts on an API for this?

Create an assortment of built-in version-matching regexes

I'd see them being perhaps in Homebrew/brew and being referenced with a symbol.

Add prefix and suffix regex options

I think this is something that could be in Homebrew/brew and build upon the built-upon ones. I don't think DRYness between formula is a priority more than explicitness and avoiding coupling.


I think this is everything I've come up so far that would affect the design/implementation of the livecheck DSL.

@samford Could you give a go at suggesting what a full DSL might look like for your options and @Moisan's?

samford commented 4 years ago

Allow for text to be replaced in version strings.

Thoughts on an API for this?

If it's possible to handle this with a block, that would seem pretty ergonomic to me. Within the block, you could do whatever you need to do with the version string, so long as it's returned in the end. For example:

livecheck do
  alterations do |version|
    version.gsub('_', '.').sub(/-rubbish$/, '')
  end
end

This would likely be used very sparingly and only for substitutions, since we can already isolate versions with a carefully crafted regex. If you need more information than this, let me know.


built-in version-matching regexes

I'd see them being perhaps in Homebrew/brew and being referenced with a symbol.

Referencing with a symbol is what I imagined as well. For example:

livecheck do
  regex :semver
end

Add prefix and suffix regex options

I think this is something that could be in Homebrew/brew and build upon the built-upon ones. I don't think DRYness between formula is a priority more than explicitness and avoiding coupling.

I'm not sure if I explained this well before, so here's an example for illustration:

livecheck do
  regex :semver,
    :prefix => "packagename-",
    :suffix => "-something"
end

If the :semver regex is /v?(\d+(?:\.\d+)+)/ in this example, this would create a regex like /packagename-v?(\d+(?:\.\d+)+)-something/. We have quite a number of livecheckables where the regex is some type of semver matching with something before/after it, so it would be nice to simplify these livecheckables in conjunction with the built-in regexes feature.

As seen in the example above, we could handle regex prefixes and suffixes as an optional hash after the regex. This makes it clear that "prefix" and "suffix" are in reference to the regex, rather than something that is prepended or appended to the version string before being returned. If this is possible from an implementation standpoint, I think this would be the way to go.

One thing to note about this: if the prefix and/or suffix options are provided as a string, we would need to escape any special characters that could be interpreted as regex syntax. For example, if the prefix was "^" and the regex was /test/, you would end up with /^test/ when you actually want /\^test/. If this would be difficult to implement, we can drop the string support and only accept regex prefix and suffix values.


Could you give a go at suggesting what a full DSL might look like...

Are you looking for something like this?

livecheck do
  skip "Not maintained"           # nil or empty string means no description
  using :git                      # May also want to accept an array of symbols
  not_using :sourceforge          # May also want to accept an array of symbols
  url "https://example.com/page"
  regex /v?(\d+(?:\.\d+)+)/,      # Accepts regex or symbol for built-in regex
    :prefix => "packagename-",    # Accepts regex or string
    :suffix => "-something"       # Accepts regex or string
  alterations do |version|
    version.gsub('_', '.').sub(/-rubbish$/, '')
  end
end

Have I missed anything or misinterpreted any of this? If everyone could review this and provide feedback when you have the time, that would be appreciated.

MikeMcQuaid commented 4 years ago

If everyone could review this and provide feedback when you have the time, that would be appreciated.

This looks great to me! In the case above I'm not sure what I'd think about using using and regex at the same time (assuming that's intended) but otherwise: perfect!

samford commented 4 years ago

I realized that I forgot to include skip in the full DSL example, so I went back and added it.

In the case above I'm not sure what I'd think about using using and regex at the same time

That last code example is just for the purpose of illustrating everything that would be available and definitely isn't meant to demonstrate a livecheckable that would actually exist in reality, ahah.

However, I can definitely imagine using and regex appearing in the same livecheckable. using will likely be pretty niche to begin with but, when it does occur, regex would seemingly be included just as often as with any other livecheckable.

My description of why this makes sense got a bit long, so I went ahead and created an issue for using in Homebrew/homebrew-livecheck. Hopefullly my explanations of the two potential applications help to make it clear that using and regex have different purposes and aren't mutually exclusive.

I'm planning to create more issues in the Homebrew/homebrew-livecheck and Homebrew/brew repos for each of these specific features when I get a chance, so we can iron out specifics of how each feature should work and discuss implementation details.

MikeMcQuaid commented 4 years ago

That last code example is just for the purpose of illustrating everything that would be available and definitely isn't meant to demonstrate a livecheckable that would actually exist in reality, ahah.

Good 😅

My description of why this makes sense got a bit long, so I went ahead and created an issue for using in Homebrew/homebrew-livecheck. Hopefullly my explanations of the two potential applications help to make it clear that using and regex have different purposes and aren't mutually exclusive.

Gotcha, read it and understand now. Agreed!

nandahkrishna commented 4 years ago

Hello. I was recently inactive due to some academic commitments, but I've been going through the codebase and think I'm getting the hang of it. I would just like to check if my understanding is right.

To formula.rb in Homebrew/Library, we would add:

  # Within class Formula
  def livecheck; end
  ...
    # Within the Formula DSLs class
    def livecheck(&block)
      define_method(:livecheck, &block)
    end

(I've completely based my guess on how test has been written. Based on my current understanding, we would define additional methods that handle the version matching and conditions like skip, etc.)

And also, in every formula inside homebrew-core's Formula/, we would be adding a livecheck block as previously suggested.

Do let me know if my intuition is right. If it is, and you give me the go-ahead, I could open a PR to start working on it. Thanks!

MikeMcQuaid commented 4 years ago

Do let me know if my intuition is right. If it is, and you give me the go-ahead, I could open a PR to start working on it. Thanks!

Sounds right as a starting point, yep!

MikeMcQuaid commented 4 years ago

Cross-posting from #7179:


It's up to you whether you match or deviate from the existing livecheck syntax in the Homebrew/homebrew-livecheck tap's brew livecheck command. What you want to be doing is making changes in three places:

  1. the Homebrew/brew repository (this one!) to add the livecheck DSL to formula.rb
  2. the Homebrew/homebrew-core repository to add livecheck to various files in Formula
  3. the Homebrew/homebrew-livecheck tap's brew livecheck command to read from Formula#livecheck/Formula.livecheck instead of the Livecheckables directory

You'll probably need to have all three PRs open at once for your local testing and we'll merge 1. first (which will cause no disruption and need no changes), then 2. for e.g. a single formula, then 3. to use the livecheck DSL if it's available or fall back to Livecheckables. When all Livecheckables have been moved to Homebrew/homebrew-core the fallback code in brew livecheck can be removed (and we may even consider migrating the command to Homebrew/brew.

samford commented 4 years ago

I've created a draft PR for a feature to filter versions based on criteria related to parts of the numeric version (e.g., a major version less than or equal to 3, a minor version less than 90, etc.). This wasn't one of the features that we originally specified but it became apparent over time that it was needed and some recent PRs relate to the issue as well. [Once this feature is merged, I'll be continuing to implement the other features but it made sense to tackle this one now.]

A full example would basically look something this for the DSL:

livecheck do
  versions {
    :comparison => :lte, # Default comparison for version parts
    :major      => 4,    # ...or { :comparison => :eq, :version => 4 }
    :minor      => 3,    # ...or { :comparison => :eq, :version => 3 }
    :patch      => 2,    # ...or { :comparison => :eq, :version => 2 }
    :other      => 1,    # ...or array of values/hashes
  }
end

If the Hash syntax doesn't need to be explicit, it could be potentially be something like:

livecheck do
  versions :comparison => :lte, # Default comparison for version parts
           :major      => 4,    # ...or { :comparison => :eq, :version => 4 }
           :minor      => 3,    # ...or { :comparison => :eq, :version => 3 }
           :patch      => 2,    # ...or { :comparison => :eq, :version => 2 }
           :other      => 1,    # ...or array of values/hashes
end

I'm looking for review/feedback on the draft PR for this feature, so I would appreciate it if anyone here who has time could take a look at Homebrew/homebrew-livecheck#536.

MikeMcQuaid commented 4 years ago
livecheck do
  versions {
    :comparison => :lte, # Default comparison for version parts
    :major      => 4,    # ...or { :comparison => :eq, :version => 4 }
    :minor      => 3,    # ...or { :comparison => :eq, :version => 3 }
    :patch      => 2,    # ...or { :comparison => :eq, :version => 2 }
    :other      => 1,    # ...or array of values/hashes
  }
end

I appreciate this is necessary but I have to be honest and say I have no idea what this means by looking at this DSL 😭. Can you break it down for an idiot like me? Thanks!

Bo98 commented 4 years ago

It seems a bit verbose, and something that could potentially limit rather than allow flexibility. For example, what if you want to restrict the minor version to be even?

In terms of homebrew-livecheck, I personally think it could be something like:

livecheck :version => ->(v) { v.major <= 4 && v.minor <= 3 }

For Homebrew/brew, the lambda could be avoided by using a block:

livecheck do
  versions do
    version.major <= 4 && version.minor <= 3
  end
end

(or equivalent notation)

samford commented 4 years ago

I appreciate this is necessary but I have to be honest and say I have no idea what this means by looking at this DSL 😭. Can you break it down for an idiot like me? Thanks!

The draft PR goes into more detail about how things work but basically that contrived example would filter out any matched versions above 4.3.2.1. The :lte symbol in the comparison corresponds to <= and would be used by default when comparing a matched Version's tokens to what's supplied in the livecheckable for filtering (e.g., is the Version's major less than or equal to 4?...is the minor less than or equal to 3?, etc.). The comparison can be overridden for a given part (e.g., minor) by passing a hash like in the comments above.

It seems a bit verbose, and something that could potentially limit rather than allow flexibility. For example, what if you want to restrict the minor version to be even?

I agree that the current draft implementation is potentially verbose and also wouldn't allow for something as seemingly simple as matching even-numbered minor versions (which is useful for most GNOME formulae). Using a proc/lambda/block would certainly improve flexibility and also simplify the implementation.

I'm already planning to use this approach for the alterations feature (above) and I successfully tested out the idea in the past, so it makes sense to me. My only concern initially was that it may be a little less user-friendly to some. That said, I'll try to work on an alternative implementation sometime in the next few days (ideally later today) and report back with the results.

It's worth noting that we don't currently have #major/#minor/#patch methods for Version, so we would be comparing against tokens like:

  livecheck :versions => -> (v) { v.tokens[0].value <= 4 && v.tokens[1].value <= 3 }

In my comment in #7340, I mentioned that the major/minor/patch meanings break down with formulae that don't use something similar to semantic versioning (e.g., YYYYMMDD version schemes like 20200409). If we wanted to have those methods, we could simply treat them like the major/minor/patch convenience API in the draft PR, where they would only be used when the meanings apply and we would simply use #tokens for non-semantic versions.

Either way, #tokens would still need to be accessible to livecheck, and #major/#minor/#patch would simply be for convenience. However, it's probably better to discuss this in #7340, as it may affect that PR.

MikeMcQuaid commented 4 years ago

The draft PR goes into more detail about how things work but basically that contrived example would filter out any matched versions above 4.3.2.1.

Why/when would you want to do that? I'm trying to understand to suggest a higher/better abstraction.

It's worth noting that we don't currently have #major/#minor/#patch methods for Version

I'd say: that's an argument for having a class other than Version to handle this stuff. Perhaps a subclass in livecheck?

Thanks!

samford commented 4 years ago

Why/when would you want to do that? I'm trying to understand to suggest a higher/better abstraction.

The general goal of the feature is to filter out versions that aren't appropriate, so livecheck doesn't erroneously list them as the newest. That is, the livecheckable regex is intended to identify all versions from a source and this feature is meant to filter out inappropriate versions. Up to now, we've dealt with this by creating complex regexes to restrict matching and this feature is intended to provide a more ergonomic way of handling this, so we can keep the regex simple (which is useful in the future, keeping the idea of built-in regexes in mind).

As before, the example was merely to illustrate the full API and not a practical real-world example. In practice, this will primarily be used to restrict matched major and minor versions but I wanted it to be flexible enough to potentially restrict any part of the version, in case we encounter any weird situations in the future that can't be easily avoided through modifying the regex.

The most common usage for now will be filtering out matched versions above (or potentially not equal to) a given major version for versioned formulae (e.g., versions above 3 for gtk3, versions above 4 for gtksourceview4, etc.). Similarly, the GNOME version scheme uses even-numbered minor versions below 90 to indicate stable releases but Homebrew/homebrew-livecheck#305 only applies this restriction when using the download.gnome.org strategy. I can create a lengthier list of formulae this would be used for (and why) but suffice to say there's a real need for this feature (e.g., we have 12 open PRs related to version restriction/filtering that I'll be revisiting after this feature is merged).

Just to be clear, this isn't intended as a way of having livecheck artificially avoid displaying a new version simply because it would be inconvenient. That is, if a formula is stuck on a given version until the formula is updated to work with a newer version (or something is addressed upstream), we would still want livecheck to display the newer version. For example, elasticsearch/kibana is currently at 6.8.7 but we wouldn't want to artificially restrict it to avoid showing 7.6.2 as the newest.

I'd say: that's an argument for having a class other than Version to handle this stuff. Perhaps a subclass in livecheck?

That's certainly a possibility, once we can access Version#tokens in that context. livecheck already extends Formula, so there's definitely precedent. However, how would this play out if livecheck (excluding livecheckables) is incorporated into Homebrew/brew as part of the DSL work? Would those extensions just be merged into Versions in the end?

MikeMcQuaid commented 4 years ago

In practice, this will primarily be used to restrict matched major and minor versions but I wanted it to be flexible enough to potentially restrict any part of the version, in case we encounter any weird situations in the future that can't be easily avoided through modifying the regex.

Let's restrict the DSL to only current use-cases. It can be extended if we need it in future (but: YAGNI).

The most common usage for now will be filtering out matched versions above (or potentially not equal to) a given major version for versioned formulae (e.g., versions above 3 for gtk3, versions above 4 for gtksourceview4, etc.). Similarly, the GNOME version scheme uses even-numbered minor versions below 90 to indicate stable releases but Homebrew/homebrew-livecheck#305 only applies this restriction when using the download.gnome.org strategy. I can create a lengthier list of formulae this would be used for (and why) but suffice to say there's a real need for this feature (e.g., we have 12 open PRs related to version restriction/filtering that I'll be revisiting after this feature is merged).

Sounds like a DSL that specifically refers to "prerelease" or "unstable" would be suitable here. There may be situations in which you want to output those versions (e.g. for a tap that tracks the development versions).

That's certainly a possibility, once we can access Version#tokens in that context. livecheck already extends Formula, so there's definitely precedent. However, how would this play out if livecheck (excluding livecheckables) is incorporated into Homebrew/brew as part of the DSL work? Would those extensions just be merged into Versions in the end?

I think there's probably still an argument for live check to have its own subclass to avoid it being exposed to/used by Formula unnecessarily.

samford commented 4 years ago

Let's restrict the DSL to only current use-cases. It can be extended if we need it in future (but: YAGNI).

If we approach this with a lambda/block like Bo suggested, then I don't think there's a difference in restrictive or expansive implementations (i.e., you can compare any parts of the version using #tokens). If you're referring to restricting the convenience methods to #major and #minor (maybe #patch), than that's fine by me. As mentioned in #7340, expecting the third token to be patch (rather than an extension, build, date, etc.) may not hold up in practice, so I'm not sure it would be valuable anyway.

At the end of the day, I'm fine with just using #tokens and I'm not personally invested in #major/#minor convenience methods. Having them may confuse some folks anyway (i.e., when do I use v.major and when do I use v.tokens[0]?).


Sounds like a DSL that specifically refers to "prerelease" or "unstable" would be suitable here. There may be situations in which you want to output those versions (e.g. for a tap that tracks the development versions).

Is it possible for a tap to maintain their own livecheckables to override ours?

Livecheck is heavily geared toward only matching stable versions and we primarily avoid other versions at the regex-level, so they're not even part of the matches. It's possible to make the versions that are filtered out as part of version-filtering feature easily available but it would take a lot of effort to make versions that are omitted due to a regex available.

Outside of this feature, being able to provide this information would require looser regexes in livecheckables (or the heuristic) and this may pick up undesirable versions other than just prerelease/unstable. You would seemingly have to develop a way of identifying prerelease/unstable versions, to separate them from stable versions and weird versions that aren't stable but also aren't prerelease/unstable, and this can vary between formulae (if it's feasible at all).

I haven't seen anyone else bring up this issue yet (I don't imagine livecheck is used much beyond us), so right now it would be a lot of work without much benefit. It may be something to revisit in the future (once livecheck is in better shape) but I don't think we should bother with it now. The modifications that only relate to this particular feature are minor and could always be done in the future.

samford commented 4 years ago

I reimplemented the version filtering feature using a lambda (per Bo's suggestion) and it only took eight lines across both heuristic.rb and utils.rb (compared to ~85 lines for the current implementation). Updating this to use a block in the future would be very easy due to the simplicity.

# heuristic.rb
if livecheck_args.is_a?(Hash) &&
   livecheck_args.key?(:versions) &&
   livecheck_args[:versions].lambda?
  filter_versions(match_version_map, livecheck_args[:versions])
end
# utils.rb
def filter_versions(match_version_map, conditions)
  match_version_map.select! { |_match, version| conditions.call(version) }
end

I've been testing this feature with a draft version of a livecheckable for gtk+ (which needs to be restricted to major versions <= 2 and even-numbered minor versions below 90) and it looks like this for the lambda setup:

class Gtkx
  livecheck :url      => "https://download.gnome.org/sources/gtk+/cache.json",
            :regex    => /gtk\+-v?(\d+(?:\.\d+)+)\.t/,
            :versions => lambda { |v|
              v.tokens[0].value <= 2 &&
                (v.tokens[1].value.even? && v.tokens[1].value < 90)
            }
end

I'm using the lambda { |v| ... } syntax here since it's a little closer to syntax for a block and will potentially make it a little easier to update livecheckable syntax when migrating to the DSL in the future.

What do we think about this? I'm partial to this setup but I'm holding off on updating the draft PR until others are on board as well.

MikeMcQuaid commented 4 years ago

Livecheck is heavily geared toward only matching stable versions and we primarily avoid other versions at the regex-level, so they're not even part of the matches.

I don't think that's a problem; Homebrew formulae similarly have a url at the top level as being the same as stable do; url ...; end and I think livechecks could do the same thing (when moved into Homebrew/brew and formulae). What I'm proposing is that as new features are added to the DSL they be readable by people who have no idea what livecheck is or what it assumes.

For example, instead of having the versions decided by a lambda: make it be stable_version_if (or something) that clarifies that the logic is reflective of finding what stable versions are.

You would seemingly have to develop a way of identifying prerelease/unstable versions, to separate them from stable versions and weird versions that aren't stable but also aren't prerelease/unstable, and this can vary between formulae (if it's feasible at all).

Yes, I'd think so. We have brew audits to do this: different versioning schemes per-package. That's another reason I'm dubious about "ignore versions above this number"; that's not going to survive a major version bump.

I haven't seen anyone else bring up this issue yet (I don't imagine livecheck is used much beyond us), so right now it would be a lot of work without much benefit. It may be something to revisit in the future (once livecheck is in better shape) but I don't think we should bother with it now. The modifications that only relate to this particular feature are minor and could always be done in the future.

I don't think brew livecheck and the tap itself need to change in any of the directions above. The reason I mention this is I do think there will need to be changes like this for the migration to include them in Homebrew/brew and formulae.

I'd suggest that we save this issue for discussion of that goal (moving code into Homebrew/brew and Homebrew/homebrew-core) and you can do whatever is desired in Homebrew/livecheck. I would bear in mind, though, that the DSL will be reviewed and tweaked as it goes into formulae so be careful that you don't add too many new features there that are going to cause issues in their migration.

Thanks for all your work on this @samford!

samford commented 4 years ago

The primary intention of this feature was to restrict the major (and minor) version for versioned formulae [without modifying the regex], so livecheck doesn't erroneously list higher versions as the newest when it's inappropriate. For example, gtkmm only deals with v2 releases and there's gtkmm3 for v3 releases but livecheck reports v3 releases as newest for gtkmm. I don't currently imagine any other applications for this feature, so we wouldn't need to worry about the check becoming incorrect after a version bump.

Filtering out certain types of unstable versions was simply another potential application if the API was flexible. Looking to a future where livecheck deals with both stable and unstable versions, it makes sense to avoid using the version filtering feature for this and to handle it differently down the road.

With the previous discussions in mind and since we only need to restrict major and minor for versioned formulae, this could be simplified down to something like:

class PhpAT73
  livecheck :major => 7,
            :minor => 3
end

This basically reads as, "Filter out matches where v.tokens[0].value != 7 && v.tokens[1].value != 3". I think this should be understandable to people unfamiliar with livecheckables (especially compared to the previous implementations of this feature).

However, this only works if we have access to a version's first and second tokens in livecheck in some way. We can't access Version#tokens in livecheck and it wasn't desirable to make them public (#7340), which is understandable. It was suggested earlier that #major and #minor methods (returning tokens[0] and tokens[1] respectively) could be handled in a subclass of Version in livecheck but I don't imagine this is possible either if livecheck can't access #tokens.

If #major and #minor methods aren't desirable inclusions in Homebrew/brew (as an alternative to making #tokens public), then I'm fine with shelving this feature until the situation changes in the future (e.g., livecheck is integrated into Homebrew/brew). I think there's value in the feature (keeping regexes simple, to allow for the possibility of using built-in, standardized regexes) but I can understand if it's technically infeasible for now or simply not seen as necessary enough to warrant inclusion in the end.

MikeMcQuaid commented 4 years ago

This basically reads as, "Filter out matches where v.tokens[0].value != 7 && v.tokens[1].value != 3". I think this should be understandable to people unfamiliar with livecheckables (especially compared to the previous implementations of this feature).

Yup, that makes sense 👍.

If #major and #minor methods aren't desirable inclusions in Homebrew/brew (as an alternative to making #tokens public), then I'm fine with shelving this feature until the situation changes in the future (e.g., livecheck is integrated into Homebrew/brew).

I would suggest subclassing in brew livecheck where you can add these methods. If this can't work: yeh, it may be worth waiting until the Homebrew/brew migration is further along.

samford commented 4 years ago

I would suggest subclassing in brew livecheck where you can add these methods. If this can't work: yeh, it may be worth waiting until the Homebrew/brew migration is further along.

Yep, sounds like it can't work for now, so let's shelve it and I'll make sure to revisit it in the future when subclassing in livecheck is feasible.

Thanks for the feedback, everyone! I'll comment again when the next feature's ready for review.

nandahkrishna commented 3 years ago

Closing this issue as the Livecheck DSL has been implemented and brew livecheck has been migrated to Homebrew/brew in #8180. Thanks everyone!

samford commented 3 years ago

The main purpose of this PR was to establish a livecheck DSL and this was accomplished in #7179 (and has since been extended to include a strategy method in #8156). Any related discussion (e.g., further additions to the livecheck DSL) can simply be handled in individual issues/PRs.

There are still some areas of the DSL that I outlined here but haven't implemented yet (e.g., alterations, regex prefix/suffix, built-in regexes, etc.). They remain on my to-do list and I plan to take care of them as time permits.