| Message ID | 20240606130102.2660-2-zachdecook@librem.one |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Zach, Thank you for the patch. On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote: > date will output a date like > 2024-06-05T17:06:30EDT Looks like posix doesn't define a %z :( I think this one is probably OK. Kieran, any opinion ? Where do the date and ln tools come from in your build environment ? > > (time zone is different than currently specified) > > ln won't create a relative link > (not a big deal because the command gets reran each time) Won't that cause issues if the directories are exported over NFS and mounted on a different absolute path ? I think the point of the Python links was to support cross-compiling on a host and running on a target device with an NFS mount. Tomi, is this correct ? > --- > specifically tested in Alpine Linux, using busybox utils > src/py/libcamera/meson.build | 4 ++-- > utils/gen-version.sh | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > index 4807ca7d..524d010c 100644 > --- a/src/py/libcamera/meson.build > +++ b/src/py/libcamera/meson.build > @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera', > # Create symlinks from the build dir to the source dir so that we can use the > # Python module directly from the build dir. > > -run_command('ln', '-fsrT', files('__init__.py'), > +run_command('ln', '-fs', files('__init__.py'), > meson.current_build_dir() / '__init__.py', > check : true) > > -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', > +run_command('ln', '-fs', meson.current_source_dir() / 'utils', > meson.current_build_dir() / 'utils', > check : true) > > diff --git a/utils/gen-version.sh b/utils/gen-version.sh > index e1f7ca7b..1b818e9e 100755 > --- a/utils/gen-version.sh > +++ b/utils/gen-version.sh > @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir") > then > git update-index --refresh > /dev/null 2>&1 > fi > -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))" > +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))" > > # If a project version is provided, use it to replace the version number. > if [ -n "$project_version" ]
On 12/06/2024 01:22, Laurent Pinchart wrote: > Hi Zach, > > Thank you for the patch. > > On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote: >> date will output a date like >> 2024-06-05T17:06:30EDT > > Looks like posix doesn't define a %z :( I think this one is probably OK. > Kieran, any opinion ? > > Where do the date and ln tools come from in your build environment ? > >> >> (time zone is different than currently specified) >> >> ln won't create a relative link >> (not a big deal because the command gets reran each time) > > Won't that cause issues if the directories are exported over NFS and > mounted on a different absolute path ? I think the point of the Python > links was to support cross-compiling on a host and running on a target > device with an NFS mount. Tomi, is this correct ? Yes. Absolute links are not nice... Tomi >> --- >> specifically tested in Alpine Linux, using busybox utils >> src/py/libcamera/meson.build | 4 ++-- >> utils/gen-version.sh | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build >> index 4807ca7d..524d010c 100644 >> --- a/src/py/libcamera/meson.build >> +++ b/src/py/libcamera/meson.build >> @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera', >> # Create symlinks from the build dir to the source dir so that we can use the >> # Python module directly from the build dir. >> >> -run_command('ln', '-fsrT', files('__init__.py'), >> +run_command('ln', '-fs', files('__init__.py'), >> meson.current_build_dir() / '__init__.py', >> check : true) >> >> -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', >> +run_command('ln', '-fs', meson.current_source_dir() / 'utils', >> meson.current_build_dir() / 'utils', >> check : true) >> >> diff --git a/utils/gen-version.sh b/utils/gen-version.sh >> index e1f7ca7b..1b818e9e 100755 >> --- a/utils/gen-version.sh >> +++ b/utils/gen-version.sh >> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir") >> then >> git update-index --refresh > /dev/null 2>&1 >> fi >> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))" >> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))" >> >> # If a project version is provided, use it to replace the version number. >> if [ -n "$project_version" ] >
Quoting Tomi Valkeinen (2024-06-12 06:54:57) > On 12/06/2024 01:22, Laurent Pinchart wrote: > > Hi Zach, > > > > Thank you for the patch. > > > > On Thu, Jun 06, 2024 at 09:01:03AM -0400, Zach DeCook wrote: > >> date will output a date like > >> 2024-06-05T17:06:30EDT > > > > Looks like posix doesn't define a %z :( I think this one is probably OK. > > Kieran, any opinion ? > > > > Where do the date and ln tools come from in your build environment ? alpine+busybox seems to support %z (though not %:z which is a more exact match) kbingham@Monstersaurus:~$ run-in-docker alpine Executing in alpine /home/kbingham # date +"%Y-%m-%dT%H:%M:%S%:z" 2024-06-12T08:56:32 /home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%:z" 2024-06-12T08:57:51 /home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S" 2024-06-12T08:58:01 /home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%z" 2024-06-12T08:58:04+0000 /home/kbingham # busybox date +"%Y-%m-%dT%H:%M:%S%Z" 2024-06-12T08:58:08UTC So if this is just about fixing/supporting alpine builds I'd go for the %z as it will sort better in the event the version string gets used to actually sort between two competing versions built at the same time but in different timezones ... but we're splitting hairs here so I don't care ;-) > >> (time zone is different than currently specified) > >> > >> ln won't create a relative link > >> (not a big deal because the command gets reran each time) > > > > Won't that cause issues if the directories are exported over NFS and > > mounted on a different absolute path ? I think the point of the Python > > links was to support cross-compiling on a host and running on a target > > device with an NFS mount. Tomi, is this correct ? > > Yes. Absolute links are not nice... > > Tomi > > >> --- > >> specifically tested in Alpine Linux, using busybox utils > >> src/py/libcamera/meson.build | 4 ++-- > >> utils/gen-version.sh | 2 +- > >> 2 files changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > >> index 4807ca7d..524d010c 100644 > >> --- a/src/py/libcamera/meson.build > >> +++ b/src/py/libcamera/meson.build > >> @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera', > >> # Create symlinks from the build dir to the source dir so that we can use the > >> # Python module directly from the build dir. > >> > >> -run_command('ln', '-fsrT', files('__init__.py'), > >> +run_command('ln', '-fs', files('__init__.py'), > >> meson.current_build_dir() / '__init__.py', > >> check : true) Busybox supports -T "Treat LINK as a file, not DIR" does that need to be dropped? (at least that sounds compatible against "-T, --no-target-directory treat LINK_NAME as a normal file always") I do worry though that the 'benefit' of these symlinks is being completley lost if the relative paths aren't maintained though. Seems like it would negate the whole point of the symlink being there at all maybe if it couldn't actually be used by the implementor... So if this patch was split, I'd be happy to merge the date fix already - but the symlink issue sounds like it needs more investigation? Presumably we'll have to have a fallback command or have the command not run at all - as the case we're trying to fix here probably aren't going to run the python scripts locally in the build... which is what I believe this symlink supports. So probably better to make the link conditional on only creating it if it's possible. > >> > >> -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', > >> +run_command('ln', '-fs', meson.current_source_dir() / 'utils', > >> meson.current_build_dir() / 'utils', > >> check : true) > >> > >> diff --git a/utils/gen-version.sh b/utils/gen-version.sh > >> index e1f7ca7b..1b818e9e 100755 > >> --- a/utils/gen-version.sh > >> +++ b/utils/gen-version.sh > >> @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir") > >> then > >> git update-index --refresh > /dev/null 2>&1 > >> fi > >> -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))" > >> +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))" > >> > >> # If a project version is provided, use it to replace the version number. > >> if [ -n "$project_version" ] > > >
On Wed Jun 12, 2024 at 5:41 AM EDT, Kieran Bingham wrote: > So if this is just about fixing/supporting alpine builds I'd go for the > %z as it will sort better in the event the version string gets used to > actually sort between two competing versions built at the same time but > in different timezones ... but we're splitting hairs here so I don't > care ;-) I was just thinking possibly to reduce the patches needed by any others (otherwise, busybox has `date -Iseconds`, but at that point patching busybox to support the longer arg name would seem more logical). > Quoting Tomi Valkeinen (2024-06-12 06:54:57) > > Yes. Absolute links are not nice... > > > > Tomi I brought back relative links in v2 using meson's fs.relative_to command. > Busybox supports -T "Treat LINK as a file, not DIR" does that need to be > dropped? (at least that sounds compatible against "-T, > --no-target-directory treat LINK_NAME as a normal file always") I kept this (where needed) in V2 > > I do worry though that the 'benefit' of these symlinks is being > completley lost if the relative paths aren't maintained though. > > Seems like it would negate the whole point of the symlink being there at > all maybe if it couldn't actually be used by the implementor... > So if this patch was split, I'd be happy to merge the date fix already V2 comes with two commits.
diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build index 4807ca7d..524d010c 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -98,11 +98,11 @@ pycamera = shared_module('_libcamera', # Create symlinks from the build dir to the source dir so that we can use the # Python module directly from the build dir. -run_command('ln', '-fsrT', files('__init__.py'), +run_command('ln', '-fs', files('__init__.py'), meson.current_build_dir() / '__init__.py', check : true) -run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', +run_command('ln', '-fs', meson.current_source_dir() / 'utils', meson.current_build_dir() / 'utils', check : true) diff --git a/utils/gen-version.sh b/utils/gen-version.sh index e1f7ca7b..1b818e9e 100755 --- a/utils/gen-version.sh +++ b/utils/gen-version.sh @@ -42,7 +42,7 @@ if [ -z "$build_dir" ] || (echo "$build_dir" | grep -q "$src_dir") then git update-index --refresh > /dev/null 2>&1 fi -git diff-index --quiet HEAD || version="$version-dirty ($(date --iso-8601=seconds))" +git diff-index --quiet HEAD || version="$version-dirty ($(date +%Y-%m-%dT%H:%M:%S%Z))" # If a project version is provided, use it to replace the version number. if [ -n "$project_version" ]