Velocity
  1. Velocity
  2. VELOCITY-502

Skip jar verification unless "force.jar.loading" is true

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5 beta2
    • Fix Version/s: 1.5
    • Component/s: Build
    • Labels:
      None
    • Environment:
      all

      Description

      When the www.iblibo.org/maven repository is down (as it seems right now, at least from here), or when you want to work form an unconnected place (yes, it still exists...), build fails because ant wants to check every jar timestamp.

      The attached patch modifies this behaviour: if a jar file is already present, the repository is not hit at all (why would we have to check any timestamp anyway since version numbers are present inside filenames?!).

      The new force.jar.loading property forces reloading of jar files.

        Activity

        Hide
        Will Glass-Husain added a comment -

        this means that we have three behaviors then?

        – normal - only download jars if missing

        – skip.jar.download - never download jars

        – force.jar.download – always download jars

        Is this right?

        Show
        Will Glass-Husain added a comment - this means that we have three behaviors then? – normal - only download jars if missing – skip.jar.download - never download jars – force.jar.download – always download jars Is this right?
        Hide
        Claude Brisson added a comment -

        Right.

        Show
        Claude Brisson added a comment - Right.
        Hide
        Henning Schmiedehausen added a comment -

        Hm. That is a very specific patch. I tried ant -Dforce.jar.loading=1 and force.jar.loading=0 with no success until I found out that it's -Dforce.jar.loading=true that works.

        It is a good and useful idea, but I'm not sure if we don't already have that semantics. We don't have them automatically, I agree.

        I'm actually -1 on that patch. I would agree to a patch that removes -Dskip.jar.loading and adds the following property and behaviour:

        velocity.jar.download

        never ---> Never go to the internet. Like skip.jar.loading= true
        once ---> Proposed behaviour. Check for presence, if missing download. If existing, do nothing.
        check ---> Current behaviour. Check for presence, if missing download, if existing, check timestamp.
        always ---> Like "force.jar.loading"

        We can argue whether "once" or "check" should be the default.

        Building the jar is a rare case for our users. They only do it if the want to debug a patch or a change. In that case, you want the build succeed and not chase a wild goose because of a stale jar. If ibiblio gets in your way, add -Dskip.jar.loading
        to your .build.properties. But I don't want to have a second (different named, booo!) property whose semantics are different from skip.jar.loading (here, presence is enough, for the proposed change the property must actually contain the 'True' value) which muddles the options that an user must go through.

        Show
        Henning Schmiedehausen added a comment - Hm. That is a very specific patch. I tried ant -Dforce.jar.loading=1 and force.jar.loading=0 with no success until I found out that it's -Dforce.jar.loading=true that works. It is a good and useful idea, but I'm not sure if we don't already have that semantics. We don't have them automatically, I agree. I'm actually -1 on that patch. I would agree to a patch that removes -Dskip.jar.loading and adds the following property and behaviour: velocity.jar.download never ---> Never go to the internet. Like skip.jar.loading= true once ---> Proposed behaviour. Check for presence, if missing download. If existing, do nothing. check ---> Current behaviour. Check for presence, if missing download, if existing, check timestamp. always ---> Like "force.jar.loading" We can argue whether "once" or "check" should be the default. Building the jar is a rare case for our users. They only do it if the want to debug a patch or a change. In that case, you want the build succeed and not chase a wild goose because of a stale jar. If ibiblio gets in your way, add -Dskip.jar.loading to your .build.properties. But I don't want to have a second (different named, booo!) property whose semantics are different from skip.jar.loading (here, presence is enough, for the proposed change the property must actually contain the 'True' value) which muddles the options that an user must go through.
        Hide
        Nathan Bubna added a comment -

        This is even better, but only so long as the 4 options are listed and described in build.properties above the default.

        My preference for the default is definitely "once", because-like Claude-i don't see any real point in checking timestamps. Version numbers should be quite sufficient.

        Show
        Nathan Bubna added a comment - This is even better, but only so long as the 4 options are listed and described in build.properties above the default. My preference for the default is definitely "once", because- like Claude -i don't see any real point in checking timestamps. Version numbers should be quite sufficient.
        Hide
        Claude Brisson added a comment -

        Le mardi 28 novembre 2006 à 12:14 -0800, Henning Schmiedehausen (JIRA) a écrit :
        > I tried ant -Dforce.jar.loading=1 and force.jar.loading=0 with no success until I found out that it's -Dforce.jar.loading=true that works.

        Ant properties allow "on","true" and "yes", but not 1... I guess you should not play lotto this week.

        > [...]
        > check ---> Current behaviour. Check for presence, if missing download, if existing, check timestamp.
        >

        Are you sure you want to keep this one? There is no point in checking timestamps of versionned filenames.

        > [...]
        > But I don't want to have a second (different named, booo!) property whose semantics are different
        > from skip.jar.loading (here, presence is enough, for the proposed change the property must actually
        > contain the 'True' value) which muddles the options that an user must go through.

        I agree that the semantics should be the same, good point (I hadn't noticed this behaviour of the "unless" attribute). But I'd rather have skip.jar.loading semantics modified, because having the property "skip.jar.loading=false" still skiping jar loading looks rather confusing.

        Also, your proposal seems much more difficult to document, whereas just putting

        skip.jar.loading = false
        force.jar.loading = false

        inside build.properties is straightforward to anybody taking a look at the file.

        Show
        Claude Brisson added a comment - Le mardi 28 novembre 2006 à 12:14 -0800, Henning Schmiedehausen (JIRA) a écrit : > I tried ant -Dforce.jar.loading=1 and force.jar.loading=0 with no success until I found out that it's -Dforce.jar.loading=true that works. Ant properties allow "on","true" and "yes", but not 1... I guess you should not play lotto this week. > [...] > check ---> Current behaviour. Check for presence, if missing download, if existing, check timestamp. > Are you sure you want to keep this one? There is no point in checking timestamps of versionned filenames. > [...] > But I don't want to have a second (different named, booo!) property whose semantics are different > from skip.jar.loading (here, presence is enough, for the proposed change the property must actually > contain the 'True' value) which muddles the options that an user must go through. I agree that the semantics should be the same, good point (I hadn't noticed this behaviour of the "unless" attribute). But I'd rather have skip.jar.loading semantics modified, because having the property "skip.jar.loading=false" still skiping jar loading looks rather confusing. Also, your proposal seems much more difficult to document, whereas just putting skip.jar.loading = false force.jar.loading = false inside build.properties is straightforward to anybody taking a look at the file.
        Hide
        Claude Brisson added a comment -

        So what do we do here?

        We have two proposals:

        1.Henning's proposal:
        velocity.jar.loading = never / once / check / always
        (where the utility of "check" is questionnable, that would let us with "never / once / always")

        2. mine:
        skip.jar.loading = true / false
        force.jar.loading = true / false
        (including changing the rather strange current behaviour of skip.jar.loading, which is to have an effect even when set with the value "false"...)

        Both are probably acceptable, and Henning's proposal is maybe more rigoureous (one parameter controlling one behaviour),
        mine is maybe more self-explanatory (but allows the strange mode skip=true & force=true, where one will discover that skip has the precedence, do we
        really care, sometimes we loose ourselves in details, it's incredible).

        Veltools has already made a step towards option 2 (but it is not irreversible). I'd just like to close this trivial issue. Option 2 is also a little easier to implement in ant. Thoughts? If nobody react here I'll commit in option 2.

        Show
        Claude Brisson added a comment - So what do we do here? We have two proposals: 1.Henning's proposal: velocity.jar.loading = never / once / check / always (where the utility of "check" is questionnable, that would let us with "never / once / always") 2. mine: skip.jar.loading = true / false force.jar.loading = true / false (including changing the rather strange current behaviour of skip.jar.loading, which is to have an effect even when set with the value "false"...) Both are probably acceptable, and Henning's proposal is maybe more rigoureous (one parameter controlling one behaviour), mine is maybe more self-explanatory (but allows the strange mode skip=true & force=true, where one will discover that skip has the precedence, do we really care, sometimes we loose ourselves in details, it's incredible). Veltools has already made a step towards option 2 (but it is not irreversible). I'd just like to close this trivial issue. Option 2 is also a little easier to implement in ant. Thoughts? If nobody react here I'll commit in option 2.
        Hide
        Nathan Bubna added a comment -

        I don't really care. Both have advantages, both will work fine. If you're going to do the work, i say you should get to decide.

        Show
        Nathan Bubna added a comment - I don't really care. Both have advantages, both will work fine. If you're going to do the work, i say you should get to decide.
        Hide
        Henning Schmiedehausen added a comment -

        Please resolve this issue if you are satisfied with the applied patch.

        Show
        Henning Schmiedehausen added a comment - Please resolve this issue if you are satisfied with the applied patch.

          People

          • Assignee:
            Henning Schmiedehausen
            Reporter:
            Claude Brisson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development