Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flags created with --define appear to be incorrectly cached #3789

Closed
hilcode opened this issue Oct 20, 2024 · 4 comments · Fixed by #3791
Closed

Flags created with --define appear to be incorrectly cached #3789

hilcode opened this issue Oct 20, 2024 · 4 comments · Fixed by #3791
Labels
bug The issue represents an bug
Milestone

Comments

@hilcode
Copy link

hilcode commented Oct 20, 2024

Given this build.sc:

import mill.T

def myProperty: T[Boolean] = T.input {
    println(s"""System.getProperty("my-property", "X")=${System.getProperty("my-property", "X")}""")
    System.getProperty("my-property", "X") match {
        case result if result == "F" || result == "X" =>
            println(s"$result ::: my-property=false")
            false

        case "T" =>
            println("T ::: my-property=true")
            true

        case x =>
            println(s"my-property='$x'")
            throw new IllegalStateException("Invalid value")
    }
}

def foo: T[String] = T {
    if (myProperty()) {
        println("TRUE")
        "TRUE"
    } else {
        println("FALSE")
        "FALSE"
    }
}

I see the following:

~/c/s/mill-bug> ./mill --version
Mill Build Tool version 0.11.12
Java version: 21.0.1, vendor: Eclipse Adoptium, runtime: /nix/store/6m2wff2lm5pk892389ivgcbs1nqbsz6v-temurin-bin-21.0.1
Default locale: en_US, platform encoding: UTF-8
OS name: "Linux", version: 6.11.3-1-default, arch: amd64

~/c/s/mill-bug> rm -rf out

~/c/s/mill-bug> ./mill --define=my-property=T foo
[build.sc] [54/59] compile
[info] compiling 1 Scala source to .../out/mill-build/compile.dest/classes ...
[info] done compiling
[1/2] myProperty
System.getProperty("my-property", "X")=T
T ::: my-property=true
[2/2] foo
TRUE

~/c/s/mill-bug> ./mill foo
[1/2] myProperty
System.getProperty("my-property", "X")=T
T ::: my-property=true

~/c/s/mill-bug> ./mill --define=my-property=F foo
[1/2] myProperty
System.getProperty("my-property", "X")=F
F ::: my-property=false
[2/2] foo
FALSE

~/c/s/mill-bug> ./mill foo
[1/2] myProperty
System.getProperty("my-property", "X")=F
F ::: my-property=false

Note that when not providing --define=my-property=?, it seems that the previous invocation of myProperty is reused somehow, including the println output.

@hilcode
Copy link
Author

hilcode commented Oct 20, 2024

Mmm, this may not be related to --define. If I use System.getenv instead of System.getProperty I get even weirder behaviour: it seems that whatever I do first is cached until I clear out.

@hilcode
Copy link
Author

hilcode commented Oct 20, 2024

import mill.T

def myProperty: T[Boolean] = T.input {
    println(s"""System.getenv("MY_PROPERTY")=${System.getenv("MY_PROPERTY")}""")
    Option(System.getenv("MY_PROPERTY")) match {
        case None | Some("F") =>
            println(s"None ::: MY_PROPERTY=false")
            false

        case Some("T") =>
            println("Some(T) ::: MY_PROPERTY=true")
            true

        case Some(x) =>
            println(s"MY_PROPERTY='$x'")
            throw new IllegalStateException("Invalid value")
    }
}

def foo: T[String] = T {
    if (myProperty()) {
        println("TRUE")
        "TRUE"
    } else {
        println("FALSE")
        "FALSE"
    }
}

With this I get:

~/c/s/mill-bug> rm -rf out

~/c/s/mill-bug> MY_PROPERTY=T ./mill foo
[build.sc] [54/59] compile
[info] compiling 1 Scala source to .../out/mill-build/compile.dest/classes ...
[info] done compiling
[1/2] myProperty
System.getenv("MY_PROPERTY")=T
Some(T) ::: MY_PROPERTY=true
[2/2] foo
TRUE

~/c/s/mill-bug> MY_PROPERTY=F ./mill foo
[1/2] myProperty
System.getenv("MY_PROPERTY")=T
Some(T) ::: MY_PROPERTY=true

~/c/s/mill-bug> MY_PROPERTY=X ./mill foo
[1/2] myProperty
System.getenv("MY_PROPERTY")=T
Some(T) ::: MY_PROPERTY=true

~/c/s/mill-bug> ./mill foo
[1/2] myProperty
System.getenv("MY_PROPERTY")=T
Some(T) ::: MY_PROPERTY=true

@lefou
Copy link
Member

lefou commented Oct 21, 2024

@lihaoyi just fixed this issue in main branch, which will become 0.12.0. We might want to backport the fix to 0.11.x too.

@lefou lefou added the bug The issue represents an bug label Oct 21, 2024
@lefou lefou added this to the 0.12.0 milestone Oct 21, 2024
lefou pushed a commit to lefou/mill that referenced this issue Oct 21, 2024
Fixes com-lihaoyi#3789. We just
accidentally flipped the conditional and didn't have any tests that
verify this behavior, so I added some example tests that also serve to
explain the feature in the docs
lefou added a commit that referenced this issue Oct 22, 2024
Fixes #3789. We just
accidentally flipped the conditional and didn't have any tests that
verify this behavior, so I added some example tests that also serve to
explain the feature in the docs.

Backport of pull request: #3791

Pull request: #3795
---------

Co-authored-by: Li Haoyi <haoyi.sg@gmail.com>
@lefou
Copy link
Member

lefou commented Oct 22, 2024

Backported to 0.11.x branch #3795 and eventually released in 0.11.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue represents an bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants