Sorry, I missed to patch the Makefile and thanks for sending a patch for that
[0].
The Makefile has another bug that I encountered during the RFC phase of the
auto installer [0]. I could include that already in a V2 or add it to the auto
installer series where the problem actually starts to show.
[0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059676.html
[1] https://lists.proxmox.com/pipermail/pve-devel/2023-September/059015.html
On 10/27/23 13:41, Christoph Heiss wrote:
Had a lot of in-person discussions with Aaron over the last few weeks
over this.
There are no real functional changes here that would impact users,
merely a code move.
Each inidivdual patch (except #1, see my answer on that) builds cleanly
on top of current master. The one dependency (see below) is also already
applied. I also did some quick smoke testing to verify everything really
still works.
I will go over the the rest of the patches afterwards, but from a glance
there is nothing that I would consider a blocker.
Things like e.g. the `InstallConfig` stuff can be done separately as
followups.
Also considering this is a great code churn, I would rather have this
get applied sooner than later, such that all future work is done on top
of this.
The only "complaint" here is that the .deb package does not build with
this series (see my reply on #1). I will shortly send a quick follow-up
patch for that, so you don't have to necessarily re-spin the whole
series just for a single, trivial line.
LGTM; thus please consider the whole series:
Reviewed-by: Christoph Heiss <c.he...@proxmox.com>
Tested-by: Christoph Heiss <c.he...@proxmox.com>
On Wed, Oct 25, 2023 at 05:59:59PM +0200, Aaron Lauterer wrote:
since work on the auto installer is happenning in parallel, now would be
a good point to move commonly used code into its own crate. Otherwise
the auto-installer will always have to play catch up with the ongoing
development of the tui installer.
I tried to split up the commits as much as possible, but there are two
larger ones, copying most the code over to the new repo and making the
switch. The former because it is difficult to pull apart the parts that
belong together. The latter needed to be this big as most local
occurences needed to be removed at the same time to avoid dependency
conflicts.
One last things that is missing, is the "InstallConfig" struct.
This should also be part of the common crate, but I need to look further
into how to make it possible that it can be created from structs of each
crate (tui, auto) as implementing a ::From can only be done within the
crate where the struct lives IIUC.
This series depends on the patches by Christoph to remove the global
unsafe setup info, version 2 [0]. Without those patches applied first,
this series will not apply.
[0] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059628.html
Aaron Lauterer (12):
add proxmox-installer-common crate
common: copy common code from tui-installer
common: utils: add dependency for doc test
common: make InstallZfsOption public
common: disk_checks: make functions public
tui-installer: add dependency for new common crate
tui: switch to common crate
tui: remove now unused utils.rs
common: add installer_setup method
common: document installer_setup method
tui: use installer_setup from common cate
tui: remove unused read_json function
Cargo.toml | 1 +
proxmox-installer-common/Cargo.toml | 12 +
proxmox-installer-common/src/disk_checks.rs | 237 ++++++++++
proxmox-installer-common/src/lib.rs | 4 +
proxmox-installer-common/src/options.rs | 387 +++++++++++++++++
proxmox-installer-common/src/setup.rs | 368 ++++++++++++++++
.../src/utils.rs | 1 +
proxmox-tui-installer/Cargo.toml | 1 +
proxmox-tui-installer/src/main.rs | 51 +--
proxmox-tui-installer/src/options.rs | 403 +-----------------
proxmox-tui-installer/src/setup.rs | 324 +-------------
proxmox-tui-installer/src/system.rs | 2 +-
proxmox-tui-installer/src/views/bootdisk.rs | 248 +----------
proxmox-tui-installer/src/views/mod.rs | 2 +-
proxmox-tui-installer/src/views/timezone.rs | 4 +-
15 files changed, 1054 insertions(+), 991 deletions(-)
create mode 100644 proxmox-installer-common/Cargo.toml
create mode 100644 proxmox-installer-common/src/disk_checks.rs
create mode 100644 proxmox-installer-common/src/lib.rs
create mode 100644 proxmox-installer-common/src/options.rs
create mode 100644 proxmox-installer-common/src/setup.rs
rename {proxmox-tui-installer => proxmox-installer-common}/src/utils.rs (99%)
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
