Expand version argument

[nikosavola]

New PR of #477 with #477 (review) addressed with dee4dfc

[hughcars]

Useful addition—the -V / --version interface and dependency output are valuable. My main concern is that source-tree probing does not provide authoritative provenance across superbuild and Spack/archive builds. I tested a producer-supplied approach here as a reference, not a requested wholesale rewrite:
https://github.com/awslabs/palace/tree/hughcars/pr775-simplified-version-metadata

[hughcars]

Thanks for taking another pass at this. The Palace version handling looks good, and passing the external source directory fixes the layout assumption. I think the remaining part is to actually pass these values from the superbuild and Spack. Right now the new interface is there, but neither caller uses it, so we still have the original problem. I’ve left examples for both.

[hughcars]

This gives the inner build somewhere to receive the version, but the superbuild never actually sends it. Since the superbuild already knows the exact tags, can we pass them here rather than finding them again from the checked-out source?

Something like:

if(PALACE_BUILD_EXTERNAL_DEPS)
  list(APPEND PALACE_OPTIONS
    "-DPALACE_DEP_mfem_VERSION=${EXTERN_MFEM_GIT_TAG}"
    "-DPALACE_DEP_libCEED_VERSION=${EXTERN_LIBCEED_GIT_TAG}"
    "-DPALACE_DEP_hypre_VERSION=${EXTERN_HYPRE_GIT_TAG}"
    "-DPALACE_DEP_metis_VERSION=${EXTERN_METIS_GIT_TAG}"
    "-DPALACE_DEP_parmetis_VERSION=${EXTERN_PARMETIS_GIT_TAG}"
  )

  if(PALACE_WITH_SLEPC)
    list(APPEND PALACE_OPTIONS
      "-DPALACE_DEP_petsc_VERSION=${EXTERN_PETSC_GIT_TAG}"
      "-DPALACE_DEP_slepc_VERSION=${EXTERN_SLEPC_GIT_TAG}"
    )
  endif()
  if(PALACE_WITH_ARPACK)
    list(APPEND PALACE_OPTIONS
      "-DPALACE_DEP_arpack_ng_VERSION=${EXTERN_ARPACK_GIT_TAG}"
    )
  endif()
  if(PALACE_WITH_SUNDIALS)
    list(APPEND PALACE_OPTIONS
      "-DPALACE_DEP_sundials_VERSION=${EXTERN_SUNDIALS_GIT_TAG}"
    )
  endif()
  if(PALACE_WITH_SUPERLU)
    list(APPEND PALACE_OPTIONS
      "-DPALACE_DEP_superlu_dist_VERSION=${EXTERN_SUPERLU_GIT_TAG}"
    )
  endif()
endif()

The same should be done for the other enabled optional dependencies. Then the source-tree lookup is actually a fallback rather than the normal route.

[nikosavola]

Addressed in 222499a

[hughcars]

We have the same gap in the Spack recipe. It knows the exact concrete dependencies, but doesn’t pass any of them through, so --version can still leave out MFEM, libCEED, PETSc, and SLEPc.

Can we add something like this in cmake_args?

dependency_versions = {
    "STRUMPACK": "strumpack",
    "arpack_ng": "arpack-ng",
    "eigen": "eigen",
    "fmt": "fmt",
    "gslib": "gslib",
    "hypre": "hypre",
    "json": "nlohmann-json",
    "libCEED": "libceed",
    "libxsmm": "libxsmm",
    "magma": "magma",
    "metis": "metis",
    "mfem": "mfem",
    "mumps": "mumps",
    "parmetis": "parmetis",
    "petsc": "petsc",
    "scalapack": "scalapack",
    "scn": "scnlib",
    "slepc": "slepc",
    "sundials": "sundials",
    "superlu_dist": "superlu-dist",
}

for cmake_name, spack_name in dependency_versions.items():
    if spack_name not in self.spec:
        continue
    dep = self.spec[spack_name]
    provenance = f"{dep.version} /{dep.dag_hash(7)}"
    args.append(
        self.define(
            f"PALACE_DEP_{cmake_name}_VERSION",
            provenance,
        )
    )

I included the short DAG hash because develop, or the same nominal dependency version, can still mean a different concrete build.

[nikosavola]

Addressed in 222499a