build: clean up the VERSION file by matthiasdiener · Pull Request #3112 · charmplusplus/charm

matthiasdiener · 2020-10-19T14:40:35Z

create two separate files, charm-version.h and charm-version-git.h in
include/, with the release/API version and git version respectively.

The files look like this:

$ cat ./netlrts-darwin-x86_64/include/charm-version.h
#define CHARM_VERSION 61002
#define CHARM_VERSION_MAJOR 6
#define CHARM_VERSION_MINOR 10
#define CHARM_VERSION_PATCH 2
$ cat ./netlrts-darwin-x86_64/include/charm-version-git.h
#define CHARM_VERSION_GIT "v6.11.0-devel-430-g50bd748ad*"

Fixes #2844.

For the old build system, commitid.sh creates the charm-version-git.h file containing the git version of Charm++ (this version was previously contained in tmp/VERSION.

create two separate files, charm-version.h and charm-api-version.h in include/, with the git version and release/API version respectively.

matthiasdiener · 2020-10-19T16:27:29Z

This is somewhat risky and the changes are not very well tested, but it fixes an important issue users have been seeing. @rbuch: Could you please check if the changes related to the tarball packaging work as expected?

nilsvu · 2020-10-19T16:23:56Z

CMakeLists.txt

 file(WRITE ${CMAKE_BINARY_DIR}/include/.vdir   "${CMK_VDIR}\n")
-file(WRITE ${CMAKE_BINARY_DIR}/include/VERSION "${CHARM_VERSION}\n")
+file(WRITE ${CMAKE_BINARY_DIR}/include/charm-version.h "#define CHARM_VERSION "${CHARM_VERSION}"\n")
+file(WRITE ${CMAKE_BINARY_DIR}/include/charm-api-version.h "#define CHARM_API_VERSION ${PROJECT_VERSION}\n")


I suggest to #define CHARM_VERSION_{MAJOR,MINOR,PATCH} as well

CMakeLists.txt

rbuch

Using $(cat charm-version.h | grep CHARM_VERSION | awk '{print $3}') in package-tarball.sh doesn't strip the version of the quotation marks, is there some other command that should be used for that? I'm confused why its use in src/scripts/Makefile doesn't result in extra quotation marks.

src/scripts/charmc

package-tarball.sh

matthiasdiener · 2020-10-19T19:44:23Z

Using $(cat charm-version.h | grep CHARM_VERSION | awk '{print $3}') in package-tarball.sh doesn't strip the version of the quotation marks, is there some other command that should be used for that? I'm confused why its use in src/scripts/Makefile doesn't result in extra quotation marks.

Good catch. Both package-tarball.sh and src/scripts/Makefile were incorrect and should be fixed now.

evan-charmworks · 2020-10-19T19:57:18Z

Should this PR add mention of VERSION's removal to CHANGES? It might be good to offer documentation on the forward transition path for any Charm users who parse it.

matthiasdiener · 2020-10-19T19:59:09Z

Should this PR add mention of VERSION's removal to CHANGES? It might be good to offer documentation on the forward transition path for any Charm users who parse it.

We could add this and also add documentation to the manual regarding CHARM_API_VERSION etc.
Another idea is to #include both files into charm-api.h or so.

matthiasdiener · 2020-10-19T20:33:02Z

Turns out that CHARM_VERSION is already used in conv-autoconfig.h (value is the same as CHARM_API_VERSION).
So I'll rename CHARM_API_VERSION_MAJOR etc. to CHARM_VERSION_MAJOR, and CHARM_VERSION to CHARM_VERSION_GIT.

rbuch · 2020-10-22T18:19:31Z

One thing to note is some applications (at least Enzo-E) read the root VERSION file as part of their build process. I don't think that should stop us from making this change, just know that it will be a breaking one.

src/scripts/configure.ac

src/ck-core/charm-api.h

matthiasdiener · 2020-10-25T02:16:44Z

Ready for review.

evan-charmworks · 2020-10-26T21:34:43Z

Does this work correctly from a packaged tarball too? I get this from CMake with current master from a tarball:

fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
==============================
-- Configuring done
Charm++ 6.10.2 configuration: 
  OS version:      Ubuntu 18.04.5 LTS (Linux 5.4.0-1031-azure, x86_64)
  Charm++ version:  (dev)

I don't know if passing --release to package-tarball.sh changes anything.

matthiasdiener · 2020-10-28T01:12:22Z

Does this work correctly from a packaged tarball too? I get this from CMake with current master from a tarball:
fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
==============================
-- Configuring done
Charm++ 6.10.2 configuration: 
  OS version:      Ubuntu 18.04.5 LTS (Linux 5.4.0-1031-azure, x86_64)
  Charm++ version:  (dev)
I don't know if passing --release to package-tarball.sh changes anything.

Good point, should be fixed now.

evan-charmworks · 2020-10-28T03:30:53Z

buildold says this:

fatal: not a git repository (or any of the parent directories): .git
Copying charm-version-git.h.new = "" over charm-version-git.h = 
./system_ln $PWD/charm-version-git.h ../include/charm-version-git.h

matthiasdiener · 2020-10-28T03:43:54Z

buildold says this:

fatal: not a git repository (or any of the parent directories): .git
Copying charm-version-git.h.new = "" over charm-version-git.h = 
./system_ln $PWD/charm-version-git.h ../include/charm-version-git.h

Ok, but this is not a regression with this PR as far as I can see.

minitu · 2020-10-28T18:42:28Z

buildold

+  rm -f charm-version.h
+  $DESTINATION/tmp/system_ln $DESTINATION/include/charm-version.h charm-version.h


Do we also want to remove charm-version-git.h here?

Not sure, but I think the rm refers to the symlink only, and there is no symlink created for charm-version-git.h.

create two separate files, charm-version.h and charm-version-git.h in include/, with the release/API version and git version respectively. The files look like this: $ cat ./netlrts-darwin-x86_64/include/charm-version.h #define CHARM_VERSION 61002 #define CHARM_VERSION_MAJOR 6 #define CHARM_VERSION_MINOR 10 #define CHARM_VERSION_PATCH 2 $ cat ./netlrts-darwin-x86_64/include/charm-version-git.h #define CHARM_VERSION_GIT "v6.11.0-devel-430-g50bd748ad*" Fixes #2844. For the old build system, commitid.sh creates the charm-version-git.h file containing the git version of Charm++ (this version was previously contained in tmp/VERSION.

build: clean up the VERSION file

dfcd384

create two separate files, charm-version.h and charm-api-version.h in include/, with the git version and release/API version respectively.

matthiasdiener added the CMake The CMake build system label Oct 19, 2020

matthiasdiener self-assigned this Oct 19, 2020

remove useless cat

6560ef0

matthiasdiener added this to the 6.11 milestone Oct 19, 2020

matthiasdiener marked this pull request as ready for review October 19, 2020 15:03

matthiasdiener requested review from evan-charmworks and rbuch October 19, 2020 15:03

matthiasdiener mentioned this pull request Oct 19, 2020

Conflict between LLVM's version file and our VERSION file #2844

Closed

nilsvu reviewed Oct 19, 2020

View reviewed changes

matthiasdiener added 2 commits October 19, 2020 11:55

a few fixes

20e6ff8

fix commitid

b490dd0

rbuch reviewed Oct 19, 2020

View reviewed changes

src/scripts/charmc Outdated Show resolved Hide resolved

package-tarball.sh Outdated Show resolved Hide resolved

matthiasdiener added 4 commits October 19, 2020 14:25

fix charm-api-version

ef0bab8

fixes to issues found by Ronak

7009899

fix commitid.sh usage in makefile

b26f8e5

fix package-tarball.sh

b9687e0

matthiasdiener added 2 commits October 19, 2020 14:59

Merge branch 'master' into version-cleanup

31e32ce

doc and #include version headers

3d37cef

matthiasdiener added 5 commits October 22, 2020 17:44

rename all

7a7a2da

fix

97ab250

Merge branch 'master' into version-cleanup

e48a741

fix charmc version

50bd748

more fixes

ae7a8be

matthiasdiener commented Oct 25, 2020

View reviewed changes

src/scripts/configure.ac Outdated Show resolved Hide resolved

matthiasdiener commented Oct 25, 2020

View reviewed changes

src/ck-core/charm-api.h Outdated Show resolved Hide resolved

matthiasdiener commented Oct 25, 2020

View reviewed changes

src/ck-core/charm-api.h Outdated Show resolved Hide resolved

matthiasdiener commented Oct 25, 2020

View reviewed changes

src/ck-core/charm-api.h Outdated Show resolved Hide resolved

matthiasdiener added 2 commits October 24, 2020 21:08

undo WS changes

f8eb9ef

clarify changes

d6f0f06

matthiasdiener requested a review from rbuch October 25, 2020 02:15

matthiasdiener requested a review from stwhite91 October 25, 2020 02:16

Merge branch 'master' into version-cleanup

783a386

matthiasdiener added 2 commits October 27, 2020 20:11

fix version when not in git repo (e.g. tarball)

f0fa75a

Merge branch 'master' into version-cleanup

f79ccdf

fix package_tarball

33b1315

evan-charmworks approved these changes Oct 28, 2020

View reviewed changes

evan-charmworks requested review from minitu and nitbhat October 28, 2020 18:35

minitu reviewed Oct 28, 2020

View reviewed changes

minitu approved these changes Oct 28, 2020

View reviewed changes

matthiasdiener merged commit adba432 into master Oct 28, 2020

matthiasdiener deleted the version-cleanup branch October 28, 2020 18:47

matthiasdiener mentioned this pull request Jan 3, 2022

charmpp: add conflict with +shared on macOS with GCC spack/spack#28211

Merged

		rm -f charm-version.h
		$DESTINATION/tmp/system_ln $DESTINATION/include/charm-version.h charm-version.h

Conversation

matthiasdiener commented Oct 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthiasdiener commented Oct 19, 2020

Uh oh!

nilsvu Oct 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rbuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

matthiasdiener commented Oct 19, 2020

Uh oh!

evan-charmworks commented Oct 19, 2020

Uh oh!

matthiasdiener commented Oct 19, 2020

Uh oh!

matthiasdiener commented Oct 19, 2020

Uh oh!

rbuch commented Oct 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthiasdiener commented Oct 25, 2020

Uh oh!

evan-charmworks commented Oct 26, 2020

Uh oh!

matthiasdiener commented Oct 28, 2020

Uh oh!

evan-charmworks commented Oct 28, 2020

Uh oh!

matthiasdiener commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minitu Oct 28, 2020

Choose a reason for hiding this comment

Uh oh!

matthiasdiener Oct 28, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

matthiasdiener commented Oct 19, 2020 •

edited

Loading

matthiasdiener commented Oct 28, 2020 •

edited

Loading