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

Fix patch version number when building from tarball #2218

Closed
wants to merge 1 commit into from

Conversation

haampie
Copy link
Contributor

@haampie haampie commented Jan 11, 2021

I have git installed on my system, and I'm using spack to build hip, which downloads tarballs without the git repo.

When building, it somehow ends up with incorrect patch version numbers (tried it on 3.9.0 ... 4.0.0).

This patch ensures that find_package(Git) will only ever be run when there is in fact a .git folder in the project source directory.

Before this patch, I ended up with shared libs like this:

$ ls /home/harmen/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-9.3.0/hip-3.9.0-bluiqfy2yt6jobgtn4qkqmdvvrqkghx6/lib/
cmake  hip_prof_str.h  libamdhip64.so  libamdhip64.so.3  libamdhip64.so.3.9.21014-

After this patch, things look much better:

$ ls /home/harmen/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-9.3.0/hip-3.9.0-4qli74twugr4mbbx7ee5niudq3oiph7x/lib/
cmake  hip_prof_str.h  libamdhip64.so  libamdhip64.so.3  libamdhip64.so.3.9.0

In fact previously I coulnd't even use the hip::amdhip64 target because it would link against /abs/path/to/libamdhip64.so.3.9.21014-, which resulted in a linking error in GCC because of the trailing dash.

@haampie
Copy link
Contributor Author

haampie commented Feb 22, 2021

Any chance this gets reviewed / merged? @pfultz2

@haampie haampie force-pushed the fix-git-available-but-no-repo branch from 9abb000 to cf58c18 Compare April 6, 2021 10:58
@haampie
Copy link
Contributor Author

haampie commented Apr 6, 2021

Bump @pfultz2

@haampie
Copy link
Contributor Author

haampie commented May 19, 2021

Any new here @pfultz2 / @yxsamliu ?

@mangupta mangupta closed this Aug 5, 2021
@mangupta mangupta reopened this Aug 5, 2021
@mangupta
Copy link
Contributor

mangupta commented Aug 5, 2021

Closing all existing pull requests that do not target the develop branch. In case this pull request is still valid, please rebase your changes against develop branch and resubmit.

@mangupta mangupta closed this Aug 5, 2021
@haampie
Copy link
Contributor Author

haampie commented Aug 5, 2021

Sorry @mangupta but this is just rude... nobody made an attempt to even look at this PR, and the only response I get is a copy & pasted message that this PR is closed.

@cgmb
Copy link

cgmb commented May 10, 2022

Thanks for the report, @haampie. I ran into the same problem when building HIP with some extra profiling flags and I was very confused. Your messages here and on the spack slack helped me understand what was wrong.

I've submitted a fix for this and a couple related issues to hipamd. You can expect the fix to arrive in a future ROCm release. I'm not sure exactly which one, but probably ROCm 5.3.

mangupta pushed a commit to ROCm/hipamd that referenced this pull request Jun 10, 2022
Prior to this change, when Git_FOUND was false, HIP_VERSION_BUILD_ID
would be undefined in the CMake code. The value of HIP_VERSION_BUILD_ID
in <hip/hip_version.h> is taken from the CMake variable, so it was being
defined as nothing in those cases. That would cause compilation failures,
as src/hip_global.cpp contains the function:

    size_t amd_dbgapi_get_build_id() {
      return HIP_VERSION_BUILD_ID;
    }

which would become

    size_t amd_dbgapi_get_build_id() {
      return ;
    }

after preprocessing. To prevent this, we can define the version
information to a default value when Git is not found.

A related problem was reported by Harmen Stoppels in
ROCm/HIP#2218. When Git
is not available (or if the library is being built from a tarball),
the HIP_VERSION_GITHASH is not defined. This causes trouble because
HIP_LIB_VERSION_STRING is defined as "X.Y.Z-${HIP_VERSION_GITHASH}"
and therefore becomes "X.Y.Z-".

The incomplete version string becomes a problem when it is appended
to the shared library file name. File names that end with '-' confuse
the linker. They cause strange errors when attempting to link to the
HIP library. This problem can be prevented by dropping the trailing
dash and using "X.Y.Z" as the version string when HIP_VERSION_GITHASH
is not defined.

Change-Id: I6e290c1f1b603ba30c9ded885e125d9ca9a2e688
Signed-off-by: Cordell Bloor <Cordell.Bloor@amd.com>
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 this pull request may close these issues.

None yet

3 participants