Fixes for Nix Builds: Timestamp Caching and Command Accumulation#465
Fixes for Nix Builds: Timestamp Caching and Command Accumulation#465pseudofractal wants to merge 2 commits intoiraf-community:mainfrom
Conversation
- Fix package file existence check using c_finfo() == ERR instead of filetime() <= 0 - Fix user file check to handle negative timestamps (usr_ftime != 0 instead of > 0) - Fix system file timestamp caching with initialization flag to prevent performance issues with pre-1980 hlib This addresses issues with Nix/reproducible builds where all files have old timestamps. Refs iraf-community#464
The previous implementation used literal '\n' strings to separate commands passed via the -c flag. This caused issues on systems where the host 'echo' did not expand the escape sequence, resulting in mangled commands like 'softools\nmkhelpdb'. This commit: 1. Replaces literal '\n' with physical newlines in the cmdline variable. 2. Uses a 'cat' heredoc to write the temporary script, ensuring newline preservation and preventing unwanted escape interpretation. 3. Ensures 'logout' is correctly recognized as a separate command to allow for clean session termination. This fixes the "task not found" and "use logout to log out" errors encountered during the helpdb and ttydata build stages.
|
On a separate note, I have been developing a set of Nix Flakes to provide a reproducible and modern installation method for IRAF. I believe this would be a valuable addition for the community and would be interested in opening a new discussion on the iraf-community organization about hosting these Flakes or integrating them as an official installation path. Could we open a discussion thread to explore how this might best serve the organization?" |
|
You can remove the Draft marker if you think it is ready; I would then start with a review. For the Nix Flakes: we can include an installation section on the IRAF installation pages (maintained in the iraf-community/iraf-community.github.io repository), just open a Pull Request there. For hosting them, I would however prefer to have it in the NixOS package repository, as we do it with Debian. |
There was a problem hiding this comment.
I think that makes it too complicated. I would recommend to define a function fileexists () next to filetime () and use that to check for the existence of pkg_pfile (where clerror () is called).
| * that is harmless. | ||
| */ | ||
| if (sys_ftime <= 0) | ||
| if (!sys_ftime_initialized) { |
There was a problem hiding this comment.
If a fileexist () is defined, this could be used here as well, ensuring that sys_ftime is not set to 0 on error.
|
Before you start adjusting the PR: It may be worth first to discuss whether the logic of pfileload () is really worth to be kept. The logic works basically like this:
This may work if the user maintains IRAF and the package directly himself. But that is not the case for pre-packaged software. F.e.: If yesterday the user created a user par file, for version (say) 2.14 and today updates IRAF (or the package) to 2.18.1 from last April: the user par file will be newer than the IRAF pkg file, no update will happen and the task may crash due to the new parameter introduced by 2.18.1. This is exactly what the logic tried to avoid. This could only be avoided if I created #466 for that which then would make that part of this PR obsolete, if we agree that this is useful. @pseudofractal can you comment there? |
Two primary blockers encountered when building and running IRAF in a Nix/sandbox environment where file timestamps are often set to the Unix Epoch (1970) or other pre-1980 dates.
unix/hlib/irafcl.shis not very robust. The -c flag accumulated commands using a literal\nstring. On many modern POSIX shells (like those used in Nix), echo did not expand these, resulting in mangled commands (e.g., softools\nmkhelpdb). Therefore I updated thegetoptsloop to use physical newlines and replacedechowith acat heredocto ensure commands are written to the temporary script file literally and sequentially. This also ensures thelogoutcommand is correctly interpreted, preventing interactive hangs. One can explore looking at;usage instead if that's preferred.The VOS logic checked if file timestamps were > 0. In reproducible build environments, file dates are often 0 (Jan 1, 1970). This caused IRAF to incorrectly assume files were missing or corrupted. Additionally, repeated unsuccessful checks for
hlib$utimecaused performance degradation. Therefore I introduced asys_ftime_initializedflag to ensure the system update time is cached even if the timestamp is 0, preventing redundant file system hits. Also updated user pfile logic to treatusr_ftime != 0as a valid state. Tried to do some fixes based on discussion from Files with pre-1980 timestamps are incorrectly treated as non-existent #464.I have verified these fixes against my Nix builds. I did some tests and IRAF CLI and PyRAF seem to be operational, successfully bypassing the previous timestamp and command-mangling blockers.
Before I undraft this PR, is there anything required to ensure this meets the standards for merging?