Preserve define variables and comma conditionals

Finding: [P2] Preserve variables declared with define — /Users/tomasz/github/t-kalinowski/quickr/R/aaa-utils.R:282-284

Resolution: define blocks now assign their collected body to the declared make variable while still ignoring body lines as standalone scanner input. Added a regression test for a define variable used by include.

Finding: [P2] Split ifeq operands at the first comma — /Users/tomasz/github/t-kalinowski/quickr/R/aaa-utils.R:375-377

Resolution: parenthesized ifeq and ifneq parsing now uses the first comma as the operand separator. Added a regression test for a comma-containing right operand.

Author: t-kalinowski

R/aaa-utils.R

@@ -246,10 +246,11 @@ quickr_makevars_scan_one_include_path <- function(path, variables, visited) {
   out <- character()
   vars <- variables
   conditional_stack <- list()
-  in_define <- FALSE
+  define_assignment <- NULL
+  define_lines <- character()
 
   for (line in lines) {
-    if (startsWith(line, "\t") && !in_define) {
+    if (startsWith(line, "\t") && is.null(define_assignment)) {
       next
     }
 
@@ -258,9 +259,14 @@ quickr_makevars_scan_one_include_path <- function(path, variables, visited) {
       next
     }
 
-    if (in_define) {
+    if (!is.null(define_assignment)) {
       if (quickr_makevars_define_end(line)) {
-        in_define <- FALSE
+        define_assignment$value <- paste(define_lines, collapse = "\n")
+        vars <- quickr_makevars_apply_assignment(vars, define_assignment)
+        define_assignment <- NULL
+        define_lines <- character()
+      } else {
+        define_lines <- c(define_lines, line)
       }
       next
     }
@@ -279,8 +285,9 @@ quickr_makevars_scan_one_include_path <- function(path, variables, visited) {
       next
     }
 
-    if (quickr_makevars_define_start(line)) {
-      in_define <- TRUE
+    define_assignment <- quickr_makevars_define_assignment(line)
+    if (!is.null(define_assignment)) {
+      define_lines <- character()
       next
     }
 
@@ -306,12 +313,23 @@ quickr_makevars_scan_one_include_path <- function(path, variables, visited) {
   list(paths = out, variables = vars)
 }
 
-quickr_makevars_define_start <- function(line) {
-  grepl(
-    "^(?:(?:export|override)[[:space:]]+)*define(?:[[:space:]]|$)",
+quickr_makevars_define_assignment <- function(line) {
+  match <- regexec(
+    "^((?:(?:export|override)[[:space:]]+)*)?define[[:space:]]+([A-Za-z_][A-Za-z0-9_.-]*).*$",
     line,
     perl = TRUE
   )
+  parts <- regmatches(line, match)[[1]]
+  if (length(parts) != 3L) {
+    return(NULL)
+  }
+
+  list(
+    name = parts[[3]],
+    operator = "=",
+    value = "",
+    override = grepl("(^|[[:space:]])override[[:space:]]+", parts[[2]])
+  )
 }
 
 quickr_makevars_define_end <- function(line) {
@@ -373,7 +391,7 @@ quickr_makevars_conditionals_active <- function(stack) {
 
 quickr_makevars_condition_result <- function(line, variables) {
   match <- regexec(
-    "^(ifeq|ifneq)[[:space:]]*\\((.*),(.*)\\)$",
+    "^(ifeq|ifneq)[[:space:]]*\\((.*?),(.*)\\)$",
     line,
     perl = TRUE
   )

tests/testthat/test-compiler-cache.R

@@ -890,6 +890,46 @@ test_that("quickr_cached_r_cmd_config_value respects Makevars conditionals", {
   expect_equal(calls, 2L)
 })
 
+test_that("quickr_cached_r_cmd_config_value splits ifeq at the first comma", {
+  cache <- new.env(parent = emptyenv())
+  root <- withr::local_tempdir()
+  active_root <- file.path(root, "active")
+  dir.create(active_root)
+  makevars <- file.path(root, "Makevars")
+  toolchain <- file.path(active_root, "toolchain.mk")
+  writeLines(
+    c(
+      "TARGET = x86,arm64",
+      "ifeq ($(TARGET),x86,arm64)",
+      paste("include", toolchain),
+      "endif"
+    ),
+    makevars
+  )
+  writeLines("FC=gfortran", toolchain)
+  calls <- 0L
+  local_mocked_bindings(
+    quickr_r_cmd_config_probe = function(name) {
+      calls <<- calls + 1L
+      list(value = paste0("value-", calls), ok = TRUE)
+    },
+    .package = "quickr"
+  )
+  withr::local_envvar(R_MAKEVARS_USER = makevars)
+
+  expect_identical(
+    quickr_cached_r_cmd_config_value("FC", cache = cache),
+    "value-1"
+  )
+
+  writeLines("FC=flang", toolchain)
+  expect_identical(
+    quickr_cached_r_cmd_config_value("FC", cache = cache),
+    "value-2"
+  )
+  expect_equal(calls, 2L)
+})
+
 test_that("quickr_cached_r_cmd_config_value respects quoted Makevars conditionals", {
   cache <- new.env(parent = emptyenv())
   root <- withr::local_tempdir()
@@ -1680,6 +1720,44 @@ test_that("quickr_cached_r_cmd_config_value ignores Makevars define bodies", {
   expect_equal(calls, 2L)
 })
 
+test_that("quickr_cached_r_cmd_config_value preserves Makevars define variables", {
+  cache <- new.env(parent = emptyenv())
+  root <- withr::local_tempdir()
+  makevars <- file.path(root, "Makevars")
+  toolchain <- file.path(root, "toolchain.mk")
+  writeLines(
+    c(
+      "define TOOLCHAIN",
+      toolchain,
+      "endef",
+      "include $(TOOLCHAIN)"
+    ),
+    makevars
+  )
+  writeLines("FC=gfortran", toolchain)
+  calls <- 0L
+  local_mocked_bindings(
+    quickr_r_cmd_config_probe = function(name) {
+      calls <<- calls + 1L
+      list(value = paste0("value-", calls), ok = TRUE)
+    },
+    .package = "quickr"
+  )
+  withr::local_envvar(R_MAKEVARS_USER = makevars)
+
+  expect_identical(
+    quickr_cached_r_cmd_config_value("FC", cache = cache),
+    "value-1"
+  )
+
+  writeLines("FC=flang", toolchain)
+  expect_identical(
+    quickr_cached_r_cmd_config_value("FC", cache = cache),
+    "value-2"
+  )
+  expect_equal(calls, 2L)
+})
+
 test_that("quickr_cached_r_cmd_config_value preserves command-line VAR", {
   cache <- new.env(parent = emptyenv())
   root <- withr::local_tempdir()