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

upstream v5.7.3 #22

Merged
merged 37 commits into from
Feb 3, 2025
Merged

upstream v5.7.3 #22

merged 37 commits into from
Feb 3, 2025

Conversation

Jo-stfc
Copy link

@Jo-stfc Jo-stfc commented Jan 30, 2025

No description provided.

smithdh and others added 30 commits December 11, 2024 19:40
This was an oversight when adding the option in commit below.

Fixes: a5cfef7
If a test causes a server to crash, a pid file with the pid of
the crashed server will be left behind. The teardown phase would
then fail due to a non-zero return of the ps command when assigning
the pid to $PID. This changes the test to skip killing a process
that has already exited without failing.
- Prefer podman instead of docker
- Remove .dockerfile which is no longer needed
- Move Dockerfiles up one directory level to simplify structure
There are coverage report generation failures on Ubuntu 24.04.
This change is to have the build working while that is sorted out.
If the client declares the expected size of the uploaded object,
then pass this information on to the OSS layer via the `oss.asize`
CGI.

This is relevant for the S3 backend as the S3 protocol requires object
sizes to be declared ahead of time.  If XRootD doesn't provide this
information, then the S3 OSS backend has to buffer the uploaded object
to know the final size, costing both in overall throughput and memory
usage.
This adds a test where `curl` is invoked in such a way that the
final object length is known at open time.  This is passed through
to the xrootd bridge layer and allows the OSS to allocate itself
appropriately.

The test itself looks through the server log not for the oss.asize
string (which is not logged) but rather for the length of the request
(which contains the URL) sent to the bridge.  The first upload of
a file does not include `oss.asize` so the size of the two log
messages can be compared.
In file included from /usr/include/features.h:524,
                 from /usr/include/c++/15/x86_64-redhat-linux/bits/os_defines.h:39,
                 from /usr/include/c++/15/x86_64-redhat-linux/bits/c++config.h:2622,
                 from /usr/include/c++/15/cstdio:46,
                 from /builddir/build/BUILD/xrootd-5.7.2-build/xrootd-5.7.2/src/XrdSut/XrdSutPFile.cc:29:
In function ‘read’,
    inlined from ‘XrdSutPFile::ReadInd(int, XrdSutPFEntInd&)’ at /builddir/build/BUILD/xrootd-5.7.2-build/xrootd-5.7.2/src/XrdSut/XrdSutPFile.cc:1525:24:
/usr/include/bits/unistd.h:32:10: error: ‘*read’ specified size 18446744073709551614 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
   32 |   return __glibc_fortify (read, __nbytes, sizeof (char),
      |          ^~~~~~~~~~~~~~~
/usr/include/bits/unistd-decl.h: In member function ‘XrdSutPFile::ReadInd(int, XrdSutPFEntInd&)’:
/usr/include/bits/unistd-decl.h:29:16: note: in a call to function ‘*read’ declared with attribute ‘access (write_only, 2, 3)’
   29 | extern ssize_t __REDIRECT_FORTIFY (__read_alias, (int __fd, void *__buf,
      |                ^~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/xrootd-5.7.2-build/xrootd-5.7.2/src/Xrd/XrdLinkCtl.cc: In function ‘Alloc’:
/builddir/build/BUILD/xrootd-5.7.2-build/xrootd-5.7.2/src/Xrd/XrdLinkCtl.cc:130:59: error: argument 1 value ‘4294967295’ exceeds maximum object size 2147483647 [-Werror=alloc-size-larger-than=]
  130 |        XrdLinkCtl **blp, *nlp = new XrdLinkCtl[LinkAlloc]();
      |                                                           ^
/usr/include/c++/15/new:140:26: note: in a call to allocation function ‘operator new []’ declared here
  140 | _GLIBCXX_NODISCARD void* operator new[](std::size_t)
      |                          ^
Uploads of new objects should have a response code of 201, not 200
See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT
Add some simple unit tests for `storage.modify` and `storage.upload`.
This commit performs a basic upload/download test, using xrdcp,
adding coverage for ZTN security and SciTokens.
This dependency is missing in gdb-minimal, which causes an error in
gdb-add-index: 'Failed to find a useable GDB binary'. The code for
gdb-add-index has been moved to use command -v in later versions,
but we need to add this change as a workaround for unfixed distros
to avoid failures when building RPMs on Alma Linux 10.

See also https://bugzilla.redhat.com/show_bug.cgi?id=2323513
Withouth this correction, `-Wall -Werror` builds on Mac fail when
importing the headers with the following message:

```
/xrootd/build/release_dir/include/xrootd/XrdSfs/XrdSfsInterface.hh:161:1: error: 'XrdSfsFSctl' defined as a struct here but previously declared as a class; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Werror,-Wmismatched-tags]
struct XrdSfsFSctl //!< SFS_FSCTL_PLUGIN/PLUGIO/PLUGXC parms
^
/xrootd/build/release_dir/include/xrootd/XrdOfs/XrdOfsFSctl_PI.hh:44:1: note: did you mean struct here?
```
ellert and others added 7 commits January 28, 2025 11:08
The format for some size_t variables were recently changed from %d to
%llu. This fixed a problem on 64 bit architectures, since there size_t
is 64 bits wide and %d is the format for a 32 bit type and %llu is the
format for a 64 bit type. However, this change introduced a regression
on 32 bit architectures where size_t is 32 bits wide.

This change caused some of the tests to fail with a segmentation fault
on the Debian armhf architecture:

The following tests FAILED:
	108 - XRootD::noauth::test (Failed)
	111 - XRootD::host::test (Failed)
	114 - XRootD::unix::test (Failed)
	117 - XRootD::sss::test (Failed)
	120 - XRootD::http::test (Failed)

This commit changes these format strings to use %zu for size_t values.
so that the compiler can do type checking for format strings
@Jo-stfc Jo-stfc merged commit 4019669 into v5.7.3patched Feb 3, 2025
18 checks passed
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.

7 participants