Copilot commented on code in PR #1758:
URL: https://github.com/apache/cloudberry/pull/1758#discussion_r3644924672


##########
devops/build/packaging/rpm/apache-cloudberry-db-incubating.spec:
##########
@@ -27,7 +50,11 @@
 %define __strip /bin/true
 %endif
 
-Name:           apache-cloudberry-db-incubating
+Name:           apache-cloudberry-db-incubating-%{cloudberry_major_version}
+# Replace the previous unversioned package on upgrade. This targets only the
+# old fixed name; it does NOT match apache-cloudberry-db-incubating-<major>,
+# so different major versions still coexist.
+Obsoletes:      apache-cloudberry-db-incubating < %{version}-%{release}
 Version:        %{version}

Review Comment:
   The package was renamed to include the major version, but the spec doesn’t 
provide the old unversioned name. This breaks in-repo RPM dependencies like 
`apache-cloudberry-hll-incubating.spec` which still has `Requires: 
apache-cloudberry-db-incubating ...`. Also, the `Obsoletes` EVR should include 
`%{?dist}` (since `Release:` does) and use `<=` so an existing 
`...-<ver>-<rel>.elX` package (or the same version being repackaged) is 
reliably replaced.



##########
devops/build/packaging/rpm/apache-cloudberry-db-incubating.spec:
##########
@@ -145,41 +177,87 @@ project has yet to be fully endorsed by the ASF.
 # No prep needed for binary RPM
 
 %build
-# No prep needed for binary RPM
+# No build needed for binary RPM
 
 %install
 rm -rf %{buildroot}
 
 # Create the versioned directory
 mkdir -p %{buildroot}%{cloudberry_install_dir}-%{version}
 
-cp -R %{cloudberry_install_dir}/* 
%{buildroot}%{cloudberry_install_dir}-%{version}
+# Use cp -a with /. to include all the files
+cp -a %{cloudberry_install_dir}/. 
%{buildroot}%{cloudberry_install_dir}-%{version}/
 
 # Copy Apache mandatory compliance files from the SOURCES directory into the 
installation directory
 cp %{_sourcedir}/LICENSE %{buildroot}%{cloudberry_install_dir}-%{version}/
 cp %{_sourcedir}/NOTICE %{buildroot}%{cloudberry_install_dir}-%{version}/
 cp %{_sourcedir}/DISCLAIMER %{buildroot}%{cloudberry_install_dir}-%{version}/
-cp -R %{_sourcedir}/licenses %{buildroot}%{cloudberry_install_dir}-%{version}/
-
-# Create the symbolic link
-ln -sfn %{cloudberry_install_dir}-%{version} 
%{buildroot}%{cloudberry_install_dir}
+cp -a %{_sourcedir}/licenses %{buildroot}%{cloudberry_install_dir}-%{version}/
 
 %files
-%{prefix}-%{version}
-%{prefix}
+%{cloudberry_install_dir}-%{version}
+%config(noreplace) %{cloudberry_install_dir}-%{version}/cloudberry-env.sh
 
 %debug_package
 
 %post
-# Change ownership to gpadmin.gpadmin if the gpadmin user exists
-if id "gpadmin" &>/dev/null; then
-    chown -R gpadmin:gpadmin %{cloudberry_install_dir}-%{version}
-    chown gpadmin:gpadmin %{cloudberry_install_dir}
+# RPM_INSTALL_PREFIX is set dynamically by RPM to the actual --prefix value.
+# Fall back to cloudberry_base_dir when --prefix was not used.
+INSTALL_PREFIX="${RPM_INSTALL_PREFIX:-%{cloudberry_base_dir}}"
+INSTALL_BASE="${INSTALL_PREFIX%/}"
+
+LINK_PATH="${INSTALL_BASE}/%{cloudberry_name}"
+VERSIONED_DIR="${INSTALL_BASE}/%{cloudberry_name}-%{version}"
+
+if [ ! -e "${LINK_PATH}" ] && [ ! -L "${LINK_PATH}" ]; then
+    # Nothing at the symlink location yet — create it.
+    ln -s "${VERSIONED_DIR}" "${LINK_PATH}" || :
+elif [ -L "${LINK_PATH}" ]; then
+    # A symlink already exists. Update it when it points to a
+    # recognized Cloudberry versioned directory.
+    EXISTING_TARGET=$(readlink -f -- "${LINK_PATH}" 2>/dev/null || :)
+    EXISTING_NAME=${EXISTING_TARGET##*/}
+
+    case "${EXISTING_NAME}" in
+        %{cloudberry_name}-*)
+            EXISTING_VERSION=${EXISTING_NAME#%{cloudberry_name}-}
+            EXISTING_MAJOR=${EXISTING_VERSION%%.*}
+            if [ "${EXISTING_MAJOR}" = "%{cloudberry_major_version}" ]; then
+                # Same major version: move the generic symlink to this build.
+                ln -sfnT "${VERSIONED_DIR}" "${LINK_PATH}" || :
+            else

Review Comment:
   The PR description says the `cloudberry-db` symlink was changed from 
absolute to relative so it remains valid after relocation, but `%post` 
currently creates/updates it with an absolute target (`${VERSIONED_DIR}`). 
Since the link and the versioned directory live in the same base directory, 
using a relative target (`cloudberry-db-%{version}`) keeps the symlink valid if 
the prefix directory is moved/copied as a unit and also aligns with the 
`%postun` removal logic that already checks the relative form.



##########
devops/build/packaging/rpm/apache-cloudberry-db-incubating.spec:
##########
@@ -145,41 +177,87 @@ project has yet to be fully endorsed by the ASF.
 # No prep needed for binary RPM
 
 %build
-# No prep needed for binary RPM
+# No build needed for binary RPM
 
 %install
 rm -rf %{buildroot}
 
 # Create the versioned directory
 mkdir -p %{buildroot}%{cloudberry_install_dir}-%{version}
 
-cp -R %{cloudberry_install_dir}/* 
%{buildroot}%{cloudberry_install_dir}-%{version}
+# Use cp -a with /. to include all the files
+cp -a %{cloudberry_install_dir}/. 
%{buildroot}%{cloudberry_install_dir}-%{version}/
 
 # Copy Apache mandatory compliance files from the SOURCES directory into the 
installation directory
 cp %{_sourcedir}/LICENSE %{buildroot}%{cloudberry_install_dir}-%{version}/
 cp %{_sourcedir}/NOTICE %{buildroot}%{cloudberry_install_dir}-%{version}/
 cp %{_sourcedir}/DISCLAIMER %{buildroot}%{cloudberry_install_dir}-%{version}/
-cp -R %{_sourcedir}/licenses %{buildroot}%{cloudberry_install_dir}-%{version}/
-
-# Create the symbolic link
-ln -sfn %{cloudberry_install_dir}-%{version} 
%{buildroot}%{cloudberry_install_dir}
+cp -a %{_sourcedir}/licenses %{buildroot}%{cloudberry_install_dir}-%{version}/
 
 %files
-%{prefix}-%{version}
-%{prefix}
+%{cloudberry_install_dir}-%{version}
+%config(noreplace) %{cloudberry_install_dir}-%{version}/cloudberry-env.sh

Review Comment:
   `%{cloudberry_install_dir}-%{version}` is a directory entry and will pull in 
the whole tree, so listing `cloudberry-env.sh` again can cause a duplicate file 
entry during `rpmbuild`. To mark it as `%config(noreplace)` without duplicating 
it, exclude it from the directory inclusion and add it back with the config 
attribute.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to