NVIDIA / hpc-container-maker

HPC Container Maker
Apache License 2.0
458 stars 94 forks source link

ucx() problem with versions with the suffix -rc #467

Closed lmagdanello closed 1 year ago

lmagdanello commented 1 year ago

What happens is that when using versions with the -rc suffix with the UCX building block, we encounter the following issue:

The module attempts to download the path: https://github.com/openucx/ucx/releases/download/v1.15.0-rc4/ucx-1.15.0-rc4.tar.gz. However, UCX provides the path for -rc versions in the following way: https://github.com/openucx/ucx/releases/download/v1.15.0-rc4/ucx-1.15.0.tar.gz.

Recipe:

Stage0 += ucx(version='1.15.0-rc4')

Log:

The command '/bin/sh -c mkdir -p /var/tmp && 
wget -q -nc --no-check-certificate -P /var/tmp **https://github.com/openucx/ucx/releases/download/v1.15.0-rc4/ucx-1.15.0-rc4.tar.gz** &&  
mkdir -p /var/tmp && tar -x -f /var/tmp/ucx-1.15.0-rc4.tar.gz -C /var/tmp -z &&     
cd /var/tmp/ucx-1.15.0-rc4 &&  
 ./configure --prefix=/usr/local/ucx 
--disable-assertions 
--disable-debug 
--disable-doxygen-doc 
--disable-logging 
--disable-params-check 
--enable-optimizations 
--without-cuda &&     
make -j$(nproc) &&     
make -j$(nproc) install &&     
rm -rf /var/tmp/ucx-1.15.0-rc4 /var/tmp/ucx-1.15.0-rc4.tar.gz' returned a non-zero code: 8
lmagdanello commented 1 year ago

My idea is that with this simple regex, we can give users the freedom to use pre-release versions.

samcmill commented 1 year ago

Does this work?

Stage0 += ucx(url='https://github.com/openucx/ucx/releases/download/v1.15.0-rc4/ucx-1.15.0.tar.gz')
lmagdanello commented 1 year ago

Yes, but in theory we could natively support pre-release versions, as they are provided in the UCX default repository.

lmagdanello commented 1 year ago

I was thinking about my suggestion and thought of another solution: perhaps the UCX building block can receive _prerelease as a parameter and if it is True it will deal with versions with the -rc suffix. What do you think?

samcmill commented 1 year ago

I am concerned about maintaining the pre-release logic.

Since there is a straightforward workaround using the url parameter, I'm inclined not to try to fix this.

lmagdanello commented 1 year ago

Of course, I understand. I'll close the issue in this case then.