-
Notifications
You must be signed in to change notification settings - Fork 568
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
bring filesystem package inline with Fedora (i.e. don't include files in the filesystem package) #8311
base: 3.0-dev
Are you sure you want to change the base?
Conversation
15e3719
to
d0dcfa4
Compare
# Default to no check, because it pulls in other packages at build, | ||
# and that will cause "circular dependency" problems for the Azure | ||
# Linux toolkit | ||
%bcond check 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to hard-code a disabled test? We shouldn't implement a separate mechanism to controlling tests runs over what we already try to do with with_check
. What happens when you use with_check
instead and remove this line? The circular dependencies sound worrying in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reduce delta from upstream, and i'd like to reenable it when/if we are not so restricted by "circular dependencies"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed a BR for tcsh
- that would be an issue for toolchain builds, I see now what you mean.
I think it would still be worth to enable the tests to run during non-toolchain builds, so we keep setup
tested. How that works, is all the BRs behind 0%{with_check}
will be ignored during the non-test build and the test build of the package will wait until its dependencies are present (BRs behind 0%{with_check}
never cause circular dependencies). The %check
failures during the toolchain build are ignored, so they shouldn't fail the build as well. Once we get testing sorted out during the toolchain phase (or we restructure the build process in another way), then the circular dependencies issue may come back, but at that point we'll need to solve it globally for all packages anyway.
d0dcfa4
to
600ab53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just marking this change as "Request Changes" to hold it until we get a stable build for preview. I'm not sure how necessary it is for that. Happy to discuss offline if you think my reasoning is incorrect.
Also drop unnecessary modprobe.d/usb.conf file.
…m package Also remove the old signature file from extended, and set the setup package to require filesystem package.
600ab53
to
9a68280
Compare
9a68280
to
c96f6ce
Compare
just waiting on your go ahead to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall generally looks good to me. Just a few comments to confirm.
@@ -1186,6 +1188,9 @@ rm -f %{name}.lang | |||
# %autochangelog. So we need to continue manually maintaining the | |||
# changelog here. | |||
%changelog | |||
* Wed Mar 06 2024 Dan Streetman <[email protected]> - 255-8 | |||
- move /var/log/* files from filesystem to systemd package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double-checking if any files need to be provided by the systemd package, since there isn't a specific /var/log/* diff here (other than the filesystem conflicts which deprecate specific files under /var/log that filesystem used to provide).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, they are actually already being provided by the current (fedora-based) systemd package, so our current situation has them being provided by both filesystem
and systemd
packages:
ddstreet [ ~ ]$ rpm -q filesystem
filesystem-1.1-19.azl3.x86_64
ddstreet [ ~ ]$ rpm -qlv filesystem|grep var/log
drwxr-xr-x 2 root root 0 Mar 8 09:18 /var/log
-rw------- 1 root root 0 Mar 8 09:18 /var/log/btmp
-rw-rw-r-- 1 root utmp 0 Mar 8 09:18 /var/log/lastlog
-rw-r--r-- 1 root root 0 Mar 8 09:18 /var/log/wtmp
ddstreet [ ~ ]$ rpm -q systemd
systemd-255-7.azl3.x86_64
ddstreet [ ~ ]$ rpm -qlv systemd|grep var/log
-rw-rw---- 1 root root 0 Mar 8 16:50 /var/log/btmp
drwxr-xr-x 2 root root 0 Mar 8 16:50 /var/log/journal
-rw-rw-r-- 1 root root 0 Mar 8 16:50 /var/log/lastlog
drwx------ 2 root root 0 Mar 8 16:50 /var/log/private
-rw-rw-r-- 1 root root 0 Mar 8 16:50 /var/log/wtmp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance containers will be writing to these folders and have issues? We don't have systemd in our containers by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't see why it would cause a problem?
c96f6ce
to
af48562
Compare
And here is the diff between our current filesystem scripts and the updated filesystem scripts:
|
Note that our mis-assignment of
|
and files diff (with each listing edited to remove differing sizes and timestamps):
|
Note that we need to fix our |
# Configuration files | ||
# | ||
cat > %{buildroot}/etc/passwd <<- "EOF" | ||
root:x:0:0:root:/root:/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is setup
creating these users now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
Move the files contained in the
filesystem
package out into the appropriate separate packages (mostly thesetup
package), and update thefilesystem
package to use Fedora packaging.Fixes: #6927