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

Descriptive error for missing git tags #1291

Closed
wihobbs opened this issue Sep 4, 2024 · 2 comments · Fixed by #1294
Closed

Descriptive error for missing git tags #1291

wihobbs opened this issue Sep 4, 2024 · 2 comments · Fixed by #1294
Assignees

Comments

@wihobbs
Copy link
Member

wihobbs commented Sep 4, 2024

@jacobtkeio was hitting this problem in the CI for his fork last week, and it took me an hour and a half to figure out why. It turns out, since his fork didn't have tags, running git push --tags made the whole world happy again.

This is a problem in flux-core too since both repositories determine version using git describe.

But, since this has come up a few times, I'm going to look in to making a better error message:

git fetch tags
  fatal: --unshallow on a complete repository does not make sense
build setup
autogen.sh
/usr/src
configure --prefix=/usr --sysconfdir=/etc --with-systemdsystemunitdir=/etc/systemd/system \
    --localstatedir=/var --prefix=/usr --sysconfdir=/etc
  Re-run cmake no build system arguments
  -- VER 0cb6cde5
  CMake Error at CMakeLists.txt:15 (project):
    VERSION "0cb6cde5" format invalid.
@wihobbs wihobbs self-assigned this Sep 4, 2024
wihobbs added a commit to wihobbs/flux-sched that referenced this issue Sep 6, 2024
Problem: flux-sched's version comes from `git describe`, which is
passed to CMake. CMake validates versions using a regex in
`CMake/Source/cmProjectCommand.cxx`. When this is an invalid
version, flux-sched can't build. This often happens in CI, forks,
or stripped-down user environments, and often happens to new
contributors.

Solution: Validate the version before passing to CMake. If the
version is invalid, throw a descriptive warning with some
suggestions.

Fixes flux-framework#1291
@wihobbs
Copy link
Member Author

wihobbs commented Sep 6, 2024

It's a different problem in core. Core will build with an invalid version, but fails on make check

In file included from version.c:15:
version.c: In function ‘flux_core_version’:
version.h:25:37: error: ‘ca5949d’ undeclared (first use in this function)
   25 | #define FLUX_CORE_VERSION_MAJOR     ca5949d
      |                                     ^~~~~~~
version.c:25:18: note: in expansion of macro ‘FLUX_CORE_VERSION_MAJOR’
   25 |         *major = FLUX_CORE_VERSION_MAJOR;
      |                  ^~~~~~~~~~~~~~~~~~~~~~~
version.h:25:37: note: each undeclared identifier is reported only once for each function it appears in
   25 | #define FLUX_CORE_VERSION_MAJOR     ca5949d
      |                                     ^~~~~~~
version.c:25:18: note: in expansion of macro ‘FLUX_CORE_VERSION_MAJOR’
   25 |         *major = FLUX_CORE_VERSION_MAJOR;
      |                  ^~~~~~~~~~~~~~~~~~~~~~~
version.h:26:37: error: ‘d’ undeclared (first use in this function)
   26 | #define FLUX_CORE_VERSION_MINOR     d
      |                                     ^
version.c:27:18: note: in expansion of macro ‘FLUX_CORE_VERSION_MINOR’
   27 |         *minor = FLUX_CORE_VERSION_MINOR;
      |                  ^~~~~~~~~~~~~~~~~~~~~~~
version.c:29:41: error: expected expression before ‘;’ token
   29 |         *patch = FLUX_CORE_VERSION_PATCH;
      |                                         ^
version.h:33:63: error: expected expression before ‘<<’ token
   33 |                                      (FLUX_CORE_VERSION_PATCH << 0))
      |                                                               ^~
version.c:30:12: note: in expansion of macro ‘FLUX_CORE_VERSION_HEX’
   30 |     return FLUX_CORE_VERSION_HEX;
      |            ^~~~~~~~~~~~~~~~~~~~~
version.c:31:1: error: control reaches end of non-void function [-Werror=return-type]
   31 | }
      | ^
cc1: all warnings being treated as errors

Autoconf takes the version of a shallow clone (depth 1) as:

checking Major version... ca5949d
checking Minor version... d
checking Point version...

@grondo
Copy link
Contributor

grondo commented Sep 6, 2024

A hash-only version should probably be rejected at configure time since it creates an invalid version.h

wihobbs added a commit to wihobbs/flux-sched that referenced this issue Sep 6, 2024
Problem: flux-sched's version comes from `git describe`, which is
passed to CMake. CMake validates versions using a regex in
`CMake/Source/cmProjectCommand.cxx`. When this is an invalid
version, flux-sched can't build. This often happens in CI, forks,
or stripped-down user environments, and often happens to new
contributors.

Solution: Validate the version before passing to CMake. If the
version is invalid, throw a descriptive warning with some
suggestions.

Fixes flux-framework#1291
@mergify mergify bot closed this as completed in #1294 Sep 9, 2024
@mergify mergify bot closed this as completed in 1bcb014 Sep 9, 2024
wihobbs added a commit to wihobbs/flux-core that referenced this issue Sep 10, 2024
Problem: autoconf will accept junk at configure time as a valid
version, then go off and generate an invalid version.h file. This
happens frequently with shallow clones, setups in CI, or other
constrained user environments. It has caused new contributors a
lot of confusion in the past.

Solution: Like flux-framework/flux-sched#1291 suggested, reject
invalid versions at configure time and provide appropriate
suggestions to the user to remedy this.
wihobbs added a commit to wihobbs/flux-core that referenced this issue Sep 10, 2024
Problem: autoconf will accept junk at configure time as a valid
version, then go off and generate an invalid version.h file. This
happens frequently with shallow clones, setups in CI, or other
constrained user environments. It has caused new contributors a
lot of confusion in the past.

Solution: Like flux-framework/flux-sched#1291 suggested, reject
invalid versions at configure time and provide appropriate
suggestions to the user to remedy this.
wihobbs added a commit to wihobbs/flux-core that referenced this issue Sep 10, 2024
Problem: autoconf will accept junk at configure time as a valid
version, then go off and generate an invalid version.h file. This
happens frequently with shallow clones, setups in CI, or other
constrained user environments. It has caused new contributors a
lot of confusion in the past.

Solution: Like flux-framework/flux-sched#1291 suggested, reject
invalid versions at configure time and provide appropriate
suggestions to the user to remedy this.
wihobbs added a commit to wihobbs/flux-core that referenced this issue Sep 11, 2024
Problem: autoconf will accept junk at configure time as a valid
version, then go off and generate an invalid version.h file. This
happens frequently with shallow clones, setups in CI, or other
constrained user environments. It has caused new contributors a
lot of confusion in the past.

Solution: Like flux-framework/flux-sched#1291 suggested, reject
invalid versions at configure time and provide appropriate
suggestions to the user to remedy this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants