Skip to content

Conversation

ddavila0
Copy link
Contributor

This adds the requirements on the major version of XRootD.
I used as a base the spec file currently used in OSG which adds few other minor changes

Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor changes requested; see comments.

BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
BuildRequires: xrootd-libs-devel xerces-c-devel pcre-devel

%define xrootd_current_major 4
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we allow this to be overridden via an RPM macro specified at build time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this would play with osg-build, could we leave that for the next time?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if 0%{?osg}
%define xrootd_current_major 4
%endif

This should be sufficient (assuming this looks right to @matyasselmeci)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the idea was to be able to set the version number at build time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what @brianhlin wrote will work. I'm not sure what you mean by "RPM macro specified at build time".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the idea was to be able to set the version number at build time

This gets us halfway there: if someone wants to build this outside of an OSG build environment, they'll have to specify xrootd_current_major. @matyasselmeci is there a way to specify xrootd_current_major as an argument to osg-build?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing arbitrary --define options to rpmbuild and mock commands is doable but not currently implemented. You can currently work around it by defining those in your ~/.rpmmacros file.

Passing --defines to Koji builds is impossible.

I strongly caution against depending on users to define specific macros via the command line, since it harms reproducibility -- you have to remember what arguments you passed to rpmbuild. If you want to change something, edit the spec file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would just be nice to specify a single spec file that can support builds against XRootD 4 and XRootD 5. This will have to do, though.


%cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo .
cd %{name}-%{version}
%cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_LIBDIR=%{_lib} .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these lines necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, this changes were in the osg svn repo for god knows how long and there is no mention of them in the changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to get rid of the cd %{name}-%{version} by changing the %setup%. -DCMAKE_INSTALL_LIBDIR=%{_lib}` is needed so that files get installed into /usr/lib64 instead of /usr/lib.

@ddavila0 ddavila0 requested a review from bbockelm July 14, 2020 22:54
@ddavila0 ddavila0 requested a review from bbockelm July 30, 2020 16:10
dfeich added a commit to dfeich/xrootd-cmstfc that referenced this pull request Apr 23, 2021
NOTE: some of these fixes may already be implemented in the pending
pull requests to the original repository. However, it contains a mix
of things, so I chose to do an own fork for the moment.
opensciencegrid#3

- enable correct library location in .../lib64/
- unpack tarball into correct version dir
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.

4 participants