Skip to content

Conversation

Crafter-Y
Copy link
Contributor

@Crafter-Y Crafter-Y commented Sep 23, 2025

Something like this is needed for itzg/docker-minecraft-server#3658.

This PR aims to add the ability to specify mcopy destinations per resource.

For example

mc-image-helper mcopy --to=.  "./downloaded<https://raw.githubusercontent.com/itzg/docker-mc-proxy/refs/heads/master/build/setup-user.sh"

and

mc-image-helper mcopy --to=. "./downloaded/one<https://raw.githubusercontent.com/itzg/docker-mc-proxy/refs/heads/master/build/setup-user.sh,./downloaded/two<https://raw.githubusercontent.com/itzg/docker-mc-proxy/refs/heads/master/build/install-packages.sh"

The prefixes are additive to the --to flag.

--to is now always used as the base path/workdir for mcopy.
If there is a absolute path given, like in the overrideDestOrder test, the base path gets overwritten
If there is a relative path, it will be based of the --to path

Any overwrite down the line should work like that:
--to -> File listing -> actual file

The --to flag is now optional. But it should error if there is no destination for any given resource.

--to gets overwritten by any destination given by a resource.
--to gets overwritten by any destination given by file listings
file listing destinations get overwritten by any destinations given by a resource

All of this does not work with manifests. If a manifest is specified by --manifest-id or --scope, a --to path must be used and there is an (handled) error if it gets overwritten.

@Crafter-Y
Copy link
Contributor Author

Hi @itzg, I know some java. but things might be a bit hacky. I ran the tests to ensure everything is still working and I added some new ones

@itzg
Copy link
Owner

itzg commented Sep 23, 2025

The --to flag is now optional. But it should error if there is no destination for any given resource.

Sorry, I meant to preemptively point out that I would like to keep the --to flag required and where it will be the base for relative paths in the per destination values. As such that can also be where the manifest is placed. For example "/data" would be the base destination and users can give paths relative to that like "config".

@Crafter-Y
Copy link
Contributor Author

Crafter-Y commented Sep 23, 2025

I reintroduced --to as you requested.

I have to admit, that I don't know how these manifests work. Since the unit tests are passing, I assume everything is working. Could you please verify that it does? I also added another unit test for that.

@itzg
Copy link
Owner

itzg commented Sep 23, 2025

Thanks. Yeah, I'll take a closer look at the manifest part. Basically they're a standard json structure I'm doing with a file listing and other data of interest for each feature, like modpack versions.

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Thanks for the update

@itzg
Copy link
Owner

itzg commented Sep 25, 2025

--to gets overwritten by any destination given by a resource.
--to gets overwritten by any destination given by file listings
file listing destinations get overwritten by any destinations given by a resource

I'm actually thinking the entry prefixes are additive to the --to and to the outer prefix when --file-is-listing. I'm suspecting the unit test scenario confused things a bit since they were absolute paths there, which would override the --to.

@Crafter-Y
Copy link
Contributor Author

The prefixes are additive to the --to flag.

--to is now always used as the base path/workdir for mcopy.
If there is a absolute path given, like in the overrideDestOrder test, the base path gets overwritten
If there is a relative path, it will be based of the --to path

Any overwrite down the line should work like that:
--to -> File listing -> actual file

@itzg
Copy link
Owner

itzg commented Sep 25, 2025

The prefixes are additive to the --to flag.

--to is now always used as the base path/workdir for mcopy.

If there is a absolute path given, like in the overrideDestOrder test, the base path gets overwritten

If there is a relative path, it will be based of the --to path

Any overwrite down the line should work like that:

--to -> File listing -> actual file

Yes, this sounds more correct.

@Crafter-Y
Copy link
Contributor Author

Thanks. Looks like we're good to go now?

@itzg
Copy link
Owner

itzg commented Sep 26, 2025

I believe so. Thanks! I'll give it one more look next time I'm at my computer and get it merged.

@itzg itzg merged commit e16ea03 into itzg:master Sep 26, 2025
1 check passed
@itzg
Copy link
Owner

itzg commented Sep 26, 2025

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