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

Move FileSystem to context manager #2428

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Conversation

schaefi
Copy link
Collaborator

@schaefi schaefi commented Jan 12, 2024

Change the FileSystem class to be a context manager. All code using FileSystem was updated to the following with statement:

with FileSystem.new(...) as filesystem:
filesystem.some_member()

This is related to Issue #2412

@schaefi schaefi requested a review from dcermak January 12, 2024 15:00
@schaefi schaefi self-assigned this Jan 12, 2024
Copy link
Collaborator

@dcermak dcermak left a comment

Choose a reason for hiding this comment

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

There are a few places that look wonky or dangerous to me as the filesystem would be guaranteed to be unmounted far earlier than it was before (where it might never be unmounted).

kiwi/builder/disk.py Outdated Show resolved Hide resolved
kiwi/builder/disk.py Outdated Show resolved Hide resolved
kiwi/builder/disk.py Show resolved Hide resolved
kiwi/builder/disk.py Show resolved Hide resolved
Change the FileSystem class to be a context manager. All code using
FileSystem was updated to the following with statement:

with FileSystem.new(...) as filesystem:
    filesystem.some_member()

This is related to Issue #2412
cherry picking merge commits is not easily possible. Thus get
the list of commits and check if it can be applied in a series
@schaefi schaefi force-pushed the move_filesystem_to_context_manager branch 3 times, most recently from 17ee68c to 075a58d Compare January 13, 2024 13:27
@schaefi
Copy link
Collaborator Author

schaefi commented Jan 13, 2024

All integration tests have built with a Staging kiwi matching this PR, see helper/build_status.sh output:

Virtualization:Appliances:Images:Testing_x86:tumbleweed
images x86_64 (published)
| images_WSL x86_64 (published)
| | openSUSE_Tumbleweed_WSL x86_64 (published)
x   .  custom-oem-boot-description
x   .  custom-pxe-boot-description
x   x  test-image-MicroOS
.   x  test-image-MicroOS:Encrypted
.   x  test-image-MicroOS:Standard
.   x  test-image-azure
.   x  test-image-bundle-format
.   x  test-image-custom-partitions
.   x  test-image-disk
.   x  test-image-disk-legacy
.   x  test-image-disk-ramdisk
.   x  test-image-disk-simple
.   x  test-image-docker
.   x  test-image-docker-derived
.   x  test-image-ec2
.   x  test-image-gce
x   x  test-image-live
.   x  test-image-live:BIOS
.   x  test-image-live:Secure
.   x  test-image-live:Standard
.   x  test-image-luks
.   x  test-image-lvm
.   x  test-image-orthos
.   x  test-image-overlayroot
.   x  test-image-partitions-and-volumes
x   x  test-image-pxe
.   x  test-image-pxe:Compressed
.   x  test-image-pxe:Flat
.   x  test-image-qcow-openstack
.   x  test-image-raid
.   x  test-image-suse-on-dnf
.   x  test-image-systemd-boot
.   x  test-image-tbz
x   x  test-image-vagrant
.   x  test-image-vagrant:libvirt
.   x  test-image-vagrant:virtualbox
. x  test-image-wsl
x   .  wsl-appx
0 1 2

Virtualization:Appliances:Images:Testing_x86:rawhide
images x86_64 (published)
.  test-image-dnf5
.  test-image-live-disk:Disk
.  test-image-live-disk:Live
.  test-image-live-disk:Virtual
.  test-image-systemd-boot
0

Virtualization:Appliances:Images:Testing_x86:leap
images x86_64 (published)
| openSUSE_Leap_15.4 x86_64 (published)
x .  kiwi-settings
. x  test-image-custom-partitions
. x  test-image-disk
. x  test-image-disk-ramdisk
. x  test-image-disk-selinux
. x  test-image-disk-simple
. x  test-image-docker
. x  test-image-docker-derived
. x  test-image-embedded:Kernel
. x  test-image-embedded:System
. x  test-image-embedded:SystemFeatures
. x  test-image-embedded:SystemOverlayRoot
. x  test-image-live
. x  test-image-luks
. x  test-image-lvm
. x  test-image-overlayroot
. x  test-image-tbz
. x  test-image-vagrant:libvirt
. x  test-image-vagrant:virtualbox
0 1

Virtualization:Appliances:Images:Testing_x86:centos
images_CentOS8 x86_64 (published)
| images_CentOS9 x86_64 (published)
x    test-image-live-disk-v8
.    test-image-live-disk-v8:Disk
.    test-image-live-disk-v8:Live
.    test-image-live-disk-v8:Virtual
x  test-image-live-disk-v9
.  test-image-live-disk-v9:Disk
.  test-image-live-disk-v9:Live
.  test-image-live-disk-v9:Virtual
0 1

Virtualization:Appliances:Images:Testing_x86:fedora
Fedora_37 x86_64 (published)
| images x86_64 (published)
. x  kiwi-settings
x .  test-image-docker
x .  test-image-live-disk:Disk
x .  test-image-live-disk:Live
x .  test-image-live-disk:Virtual
x .  test-image-microdnf
0 1

Virtualization:Appliances:Images:Testing_x86:ubuntu
images x86_64 (published)
.  test-image-docker
.  test-image-live-disk:Disk
.  test-image-live-disk:Live
.  test-image-live-disk:Virtual
0

Virtualization:Appliances:Images:Testing_arm:ubuntu
images aarch64 (published)
.  test-image-rpi
0

Virtualization:Appliances:Images:Testing_x86:debian
images x86_64 (published)
.  test-image-live-disk:Disk
.  test-image-live-disk:Live
.  test-image-live-disk:Virtual
0

Virtualization:Appliances:Images:Testing_s390:tumbleweed
images s390x (building)
| openSUSE_Tumbleweed_zSystems s390x (blocked)
x b  kiwi-settings
s x  test-image-disk:Physical
s x  test-image-disk:Virtual
0 1
Virtualization:Appliances:Images:Testing_s390:sle15
Virtualization:Appliances:Images:Testing_arm:tumbleweed
images aarch64 (published)
.  test-image-live-disk:Disk
.  test-image-live-disk:Live
.  test-image-rpi
.  test-image-rpi-overlay
0

Virtualization:Appliances:Images:Testing_arm:fedora
Fedora_37 aarch64 (published)
| images aarch64 (published)
. x  kiwi-settings
x .  test-image-live
0 1

Virtualization:Appliances:Images:Testing_ppc:tumbleweed
images ppc64le (published)
.  test-image-disk-simple
.  test-image-disk:PhysicalBSZ_4096
.  test-image-disk:PhysicalBSZ_512
0

Virtualization:Appliances:Images:Testing_ppc:sle15
Virtualization:Appliances:Images:Testing_x86:archlinux
images x86_64 (published)
| standard x86_64 (published)
x .  dracut-hooks
. x  test-image-live-disk-kis:Disk
. x  test-image-live-disk-kis:KIS
. x  test-image-live-disk-kis:Live
. x  test-image-live-disk-kis:Virtual
0 1

Legend:
 . succeeded
 D Disabled
 U unresolvable
 F failed
 B broken
 b blocked
 % building
 f finished
 s scheduled
 L locked
 x excluded
 d dispatching
 S signing
 ? buildstatus not available

Use proper Union declaration for system variable and add
consistency layer into Filesystem/VolumeManager classes to
meet the type declaration as well as to simplify further
refactoring on these classes
@schaefi schaefi force-pushed the move_filesystem_to_context_manager branch from 4aca8a2 to abac4d0 Compare January 15, 2024 15:49
@schaefi schaefi merged commit 31e6f2d into main Jan 16, 2024
15 checks passed
@schaefi schaefi deleted the move_filesystem_to_context_manager branch January 16, 2024 12:51
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.

2 participants