guysoft / CustomPiOS

A Raspberry Pi and other ARM devices distribution builder
GNU General Public License v3.0
508 stars 146 forks source link

allow distro-relative paths in custompios_path #206

Open umlaeute opened 1 year ago

umlaeute commented 1 year ago

the <dist>/src/custompios_path generated by src/make_custom_pi_os holds the absolute path to the CustomPiOS sources (so all the scripts (and sourced scriptlets) can find the other scripts).

this means, that it is useless to track this file in git (assuming that not everybody clones the repo into /home/guysoft/sources/CustomPiOS :-P)

probably this file could hold a relative path (at least if make_custom_pi_os was called via a relative path, such as ./src/make_custom_pi_os)?

i'm a bit unsure about the interaction with the vagrant snippet.

guysoft commented 1 year ago

The relative path becomes a problem if you are using docker build environment. Or multiple CustomPiOS source trees.

I am not sure how to solve this without placing custompios in the execution PATH variable. So this was the compromise at the time.

What exactly are you proposing with relative paths? Relative to the working directory or the distro you are building?

umlaeute commented 1 year ago

i think that relative paths to the distro is what I mean.

e.g.

$ .
├── LICENSE
├── README.md
├── mydistro
│   └── src
│       ├── build_dist
│       ├── config
│       └── custompios_path
└── src
    ├── build
    ├── build_custom_os
    ├── build_docker
    ├── common.sh
    ├── config
    ├── config_sanity
    ├── custompios
    ├── make_custom_pi_os
    └── update-custompios-paths

$ cat mydistro/src/custompios_path
../../src

this obviously assumes that it actually makes sense to use a relative path, because the mydistro directory and the CustomPiOS/src folder are guaranteed to have a fixed relative path (e.g. because both are tracked in the same git repository). this is my current mode of operation (but I'm sure others do it differently).

it probably makes sense to have src/update-custompios-paths generate absolute paths, but allow relative paths. right now it fails with:

python3: can't open file '<<CUSTOMPIOS_SRCDIR>>/mydistro/src/workspace/mount/../../src/execution_order.py': [Errno 2] No such file or directory
umlaeute commented 1 year ago

sidenote: my current workaround to this problem is to exclude the mydist/src/custompios_path from my repository, and instead have a wrapper-script that first creates this file (with the absolute path) and then call mydistro's build_dist.

so the priority of this issue is pretty low for me.

umlaeute commented 1 year ago

i guess this might work:

diff --git a/src/dist_generators/dist_example/src/build_dist b/src/dist_generators/dist_example/src/build_dist
index e3d3cb8..14799a7 100755
--- a/src/dist_generators/dist_example/src/build_dist
+++ b/src/dist_generators/dist_example/src/build_dist
@@ -5,4 +5,6 @@ export DIST_PATH=${DIR}
 export CUSTOM_PI_OS_PATH=$(<${DIR}/custompios_path)
 export PATH=$PATH:$CUSTOM_PI_OS_PATH

+CUSTOM_PI_OS_PATH="$( cd "$(DIR}" && cd "${CUSTOM_PI_OS_PATH}" && pwd )"
+
 ${CUSTOM_PI_OS_PATH}/build_custom_os $@

afaict, the custompios_path file is only read once (in the distro's build_dist script) so it could be expanded to an absolute path right there.

this still doesn't handle the expansion in Vagrantfile though (I'm afraid my ruby-knowledge is nil)

umlaeute commented 1 year ago

i guess that's the change required for the Vagrantfile:

diff --git a/src/dist_generators/dist_example/src/vagrant/Vagrantfile b/src/dist_generators/dist_example/src/vagrant/Vagrantfile
index 4484441..f6bfa37 100644
--- a/src/dist_generators/dist_example/src/vagrant/Vagrantfile
+++ b/src/dist_generators/dist_example/src/vagrant/Vagrantfile
@@ -2,7 +2,7 @@ vagrant_root = File.dirname(__FILE__)
 Vagrant.configure("2") do |o|
     o.vm.box = "debian/buster64"
     o.ssh.shell = "bash -c 'BASH_ENV=/etc/profile exec bash'"
-    o.vm.synced_folder File.read("../custompios_path").gsub("\n",""), "/CustomPiOS", create:true, type: "nfs"
+    o.vm.synced_folder File.expand_path(File.read("../custompios_path").gsub("\n", ""), ".."), "/CustomPiOS", create:true, type: "nfs"
     o.vm.synced_folder "../", "/distro", create:true, type: "nfs"
     o.vm.network :private_network, ip: "192.168.55.55"
     o.vm.provision :shell, :path => "setup.sh", args: ENV['SHELL_ARGS']

note that i haven't actually tested this with Vagrant (just with a small, standalone ruby script)

guysoft commented 1 year ago

Its part of .gitignore on my distros: eg: https://github.com/guysoft/OctoPi/blob/devel/.gitignore#L2

I initially didn't add git files in to the template folder of a new distro. But since 2013 when it started git has become the only version control out there. Though there are a few that use mercurial (though I rarely see .hgignore). So perhaps it might make sense to add now a default .gitignore.

ATM the vagrant is falling out of support no one is actually using it. And I have no time to test it.

umlaeute commented 1 year ago

I initially didn't add git files in to the template folder of a new distro.

oops: with b10ed770de2dab48db45e92e388446b096ebadfe i have introduced a default .gitignore file (and continued to work on that with 25bff8b08810755ba865f2db77774c9dd4c15bcb).

so probably, you could just keep it and add more files?

and what do you think of https://github.com/guysoft/CustomPiOS/issues/206#issuecomment-1743436817?

one potential drawback i see is, that if the path in custompios_path does not exist (regardless of absolute or relative), the CUSTOM_PI_OS_PATH will then hold the same value as DIR. this might give confusing error messages. using realpath instead of the cd ... && pwd magic would fix this, but i guess it is not very portable (e.g. wouldn't run on macOS)