Skip to content

feat: add support for some YaST-like URLs#2118

Merged
imobachgs merged 33 commits intoafter-release-beta2from
extend-transfer-urls
Mar 6, 2025
Merged

feat: add support for some YaST-like URLs#2118
imobachgs merged 33 commits intoafter-release-beta2from
extend-transfer-urls

Conversation

@imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Mar 5, 2025

Problem

Agama Transfer's API does not understand YaST-specific URLs.

It is impossible to handle URLs like usb://OEMDRV/autoinst.xml, device:///autoinst.xml, etc.

Note

It might not cover 100% of the old API cases, but we are working towards that.

Solution

This PR aims to add partial support for these URLs. The following schemes are supported: device:, usb:, label:, HD:, DVD: and cd:. Support for well-known URLs (e.g., file:, http:, https:, ftp:, nfs:, etc.) is still implemented using CURL.

Warning

agama download now requires specifying a DESTINATION file (instead of writing to stdout). Agama uses stdout to tell where it is searching.

Testing

  • Added a new unit test
  • Tested manually

imobachgs and others added 26 commits February 28, 2025 13:06
Temporarily disable signature checking for dir:// because with the new
product composer it is not possible to sign the repositories yet. It
will be fixed in the future and we should remove this code then.

See #2092 for further information.

## Testing

* Add a unit test.
* Manually tested.
Resync patterns list for SLES / SLES_SAP 16
## Problem

QA tests of `agama profile import` fail because we did not package the
storage part of the schema

- https://bugzilla.suse.com/show_bug.cgi?id=1238367#c13

## Solution

1. Package that file.
2. and remove the dangling reference to `guided`


## Testing

- *Added a new unit test*
- *Tested manually*


## Screenshots

*If the fix affects the UI attach some screenshots here.*
Once the problem with the produce composer is fixed (see #2092), it is
time to enable the signature
checking again for `dir://` repositories.
@imobachgs imobachgs marked this pull request as ready for review March 6, 2025 10:17
let source = mount_point.join(&file_name);
let result = Self::copy_file(source, writer);

if !file_system.is_mounted() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this basically means that even if previously file system was already mounted, it will be unmounted here, which can be surprising and buggy. What about using closure like guarantee for mount that will handle it like

let file_name = file_name.strip_prefix("/").unwrap_or(file_name);
let source = mount_point.join(&file_name);
let result = file_system.ensure_mounted(&mount_point, || Self::copy_file(source, writer))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it will not umount the file system. The is_mounted() retains the original value, so there is no danger.

"--path",
])
.output()
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

this deserve at least todo for failures to report it more nicely.

@imobachgs imobachgs changed the base branch from master to after-release-beta2 March 6, 2025 12:45
@imobachgs imobachgs merged commit 9b86fd2 into after-release-beta2 Mar 6, 2025
1 check passed
@imobachgs imobachgs deleted the extend-transfer-urls branch March 6, 2025 13:17
@coveralls
Copy link

coveralls commented Mar 6, 2025

Pull Request Test Coverage Report for Build 13699080004

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 37 of 156 (23.72%) changed or added relevant lines in 8 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.3%) to 72.479%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rust/agama-lib/src/scripts/model.rs 0 1 0.0%
rust/agama-cli/src/profile.rs 0 2 0.0%
rust/agama-lib/src/utils/transfer.rs 0 7 0.0%
rust/agama-cli/src/lib.rs 0 8 0.0%
rust/agama-lib/src/utils/transfer/handlers/generic.rs 0 9 0.0%
rust/agama-lib/src/utils/transfer/file_finder.rs 0 15 0.0%
rust/agama-lib/src/utils/transfer/file_systems.rs 37 73 50.68%
rust/agama-lib/src/utils/transfer/handlers/yast.rs 0 41 0.0%
Files with Coverage Reduction New Missed Lines %
rust/agama-cli/src/lib.rs 1 0.0%
rust/agama-server/tests/common/mod.rs 5 74.19%
Totals Coverage Status
Change from base Build 13680801738: -0.3%
Covered Lines: 20010
Relevant Lines: 27608

💛 - Coveralls

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