all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
@ 2024-08-13 12:45 Nigko Yerden
  2024-08-13 14:38 ` pelzflorian (Florian Pelz)
  2024-08-14 14:00 ` Attila Lendvai
  0 siblings, 2 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-13 12:45 UTC (permalink / raw)
  To: guix-devel

Hello Guix!,

Consider a minimal test git repository [1] created in line with Cookbook
recommendations [2]. It has the following file structure:
      .
      ├── content
      ├── .guix-channel
      ├── guix.scm → .guix/modules/test-repo-package.scm
      └── .guix
          └── modules
             └── test-repo-package.scm

Here 'content' is a text file. '.guix-channel' includes
-----------------begin------------------------------------------------------------
(channel
   (version 0)
   (directory ".guix/modules"))
-----------------end--------------------------------------------------------------

and '.guix/modules/test-repo-package.scm' provides a package definition:
-----------------begin------------------------------------------------------------
(define-module (test-repo-package)
   #:use-module (guix packages)
   #:use-module (guix gexp)
   #:use-module (guix utils)
   #:use-module (guix git-download)
   #:use-module (guix build-system copy)
   #:use-module (guix licenses))

(define vcs-file?
   ;; Return true if the given file is under version control.
   (or (git-predicate (dirname (dirname (current-source-directory))))
       (const #t)))

(define-public test-repo
   (package
     (name "test-repo")
     (version "1.0")
     (source (local-file "../.." "test-repo-checkout"
			#:recursive? #t
			#:select? vcs-file?))
     (build-system copy-build-system)
     (arguments
      (list #:install-plan #~'(("content" "share/"))))
     (synopsis "Example: git repo as a package")
     (description "Example: git repo as a package")
     (home-page "https://example.com")
     (license gpl3+)))

test-repo
-----------------end--------------------------------------------------------------

All what this package does is to install 'content' file to
'/gnu/store/..../share/content' path. 'guix build -f guix.scm' command works as
expected. However, if we add the repository to the list of channels in
~/.config/guix/channels.scm file:
-----------------begin------------------------------------------------------------
(list ....
  (channel
   (name 'test-channel)
   (url "https://gitlab.com/anigko/test-channel.git")
   (branch "main")))
-----------------end--------------------------------------------------------------

and run 'guix pull', the command 'guix build test-repo' will fail with an error
message "No such file or directory 'content'" unless your GUILE_LOAD_PATH does
not include '~/.config/guix/current/share/guile/site/3.0/' path (this is the
path where a symlink to 'test-repo-package.scm' is installed by 'guix pull').

Normally GUILE_LOAD_PATH does include above-mentioned path. Indeed,
the GUIX System installer injects the following snippet
-----------------begin------------------------------------------------------------
eval "$(guix package --search-paths \
-p $HOME/.config/guix/current \
-p $HOME/.guix-profile \
-p /run/current-system/profile)"
-----------------end--------------------------------------------------------------
into '~/.bash_profile' file, setting many environment variables, and GUILE_LOAD_PATH
in particular. In this case '(local-file "../.." "test-repo-checkout" ...)' expression
is run from '~/.config/guix/current/share/guile/site/3.0/test-repo-package.scm' file,
which is a symlink. But '(local-file "../.." ...)' does not follow this symlink,
and the 'source' field of 'test-repo' package is evaluated to
'~/.config/guix/current/share/guile/site/', which is wrong of course.


Here is a workaround for this behavior. From the definition of 'local-file' in
guix/gexp.scm one can deduce that the executed relevant code is:

(absolute-file-name "../.." (current-source-directory))

Here '(current-source-directory)' evaluates to '~/.config/guix/current/share/guile/site/3.0/'.
However, if the symlink in '3.0/' directory would not target a file but another directory,
say 'test-repo', containing a file 'package.scm' with the package definition,
then '(current-source-directory)' will follow the symlink, that is what we want.

The branch 'alt' of [1] provides a realization of the workaround:
      .
      ├── content
      ├── .guix-channel
      ├── guix.scm → .guix/modules/test-repo/package.scm
      └── .guix
          └── modules
             └── test-repo
               └── package.scm

In comparison with 'test-repo-package.scm' file from 'main' branch, the 'package.scm'
file contains three modifications:

1.
(define-module (test-repo package)

2.
(define vcs-file?
   ;; Return true if the given file is under version control.
   (or (git-predicate (dirname (dirname (dirname (current-source-directory)))))
       (const #t)))

3.
     (source (local-file "../../.." "test-repo-checkout"
			#:recursive? #t
			#:select? vcs-file?))

Thus defined repository ensures that 'test-repo' package is built without errors on
systems with and without properly configured GUILE_LOAD_PATH.

Regards,
Nigko

[1] https://gitlab.com/anigko/test-channel.git
[2] https://guix.gnu.org/en/cookbook/en/html_node/The-Repository-as-a-Channel.html


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 12:45 Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH Nigko Yerden
@ 2024-08-13 14:38 ` pelzflorian (Florian Pelz)
  2024-08-13 15:06   ` Nigko Yerden
  2024-08-14 14:00 ` Attila Lendvai
  1 sibling, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-13 14:38 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: guix-devel

Hello Nigko.  Do I understand correctly: Your grievance is the
definition of `local-file', which for relative paths like "../.." does
not follow symlinks to non-directory files, but should?  And this is
because current-source-directory returns the wrong path in a Guix
channel?

That is, `local-file' does not work correctly?

And the problem is not how the cookbook is written?


Could you create a bug report by writing a mail to

bug-guix@gnu.org

with this concrete issue, referencing this your mail to guix-devel:

https://lists.gnu.org/archive/html/guix-devel/2024-08/msg00047.html


Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 14:38 ` pelzflorian (Florian Pelz)
@ 2024-08-13 15:06   ` Nigko Yerden
  2024-08-13 17:25     ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-13 15:06 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: guix-devel

Hello Florian,

There is no grievance for anything. Sorry, if my message has such
a tinge, it does not correspond to what was meant. Also I do not
think that 'local-file' is doing something wrong. I suspect that
introducing changes to such an ubiquitous procedure on such a minor
subject is not necessary a good thing. What I meant is that recipe
from cookbook does not work for Guix with GUILE_LOAD_PATH configured
as in freshly installed Guix System, nothing more.

Regards,
Nigko
  

pelzflorian (Florian Pelz) wrote:
> Hello Nigko.  Do I understand correctly: Your grievance is the
> definition of `local-file', which for relative paths like "../.." does
> not follow symlinks to non-directory files, but should?  And this is
> because current-source-directory returns the wrong path in a Guix
> channel?
> 
> That is, `local-file' does not work correctly?
> 
> And the problem is not how the cookbook is written?
> 
> 
> Could you create a bug report by writing a mail to
> 
> bug-guix@gnu.org
> 
> with this concrete issue, referencing this your mail to guix-devel:
> 
> https://lists.gnu.org/archive/html/guix-devel/2024-08/msg00047.html
> 
> 
> Regards,
> Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 15:06   ` Nigko Yerden
@ 2024-08-13 17:25     ` pelzflorian (Florian Pelz)
  2024-08-13 17:49       ` Nigko Yerden
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-13 17:25 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: guix-devel

Ahh I had misunderstood.  Your intention was just that others with the
same use-case find your workaround in the mail archives.  Then you did
right.  The guix-cookbook describes a (local-file "."), not "../..", so
I guess it needs no changes, or should we add a hint not to ..?

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 17:25     ` pelzflorian (Florian Pelz)
@ 2024-08-13 17:49       ` Nigko Yerden
  2024-08-14 15:35         ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-13 17:49 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: guix-devel

pelzflorian (Florian Pelz) wrote:
> Your intention was just that others with the
> same use-case find your workaround in the mail archives.  Then you did
> right.  
Not just this, but also that somebody may consider changing the cookbook.


> The guix-cookbook describes a (local-file "."), not "../..",
You are looking at "7.2 Level 1: Building with Guix".
See "7.2 Level 2: The Repository as a Channel",
https://guix.gnu.org/en/cookbook/en/html_node/The-Repository-as-a-Channel.html,
where there is a "../.." in the 'local-file' argument.

> I guess it needs no changes, or should we add a hint not to ..?
I think it needs some changes, a warning at least. Also I think that
keeping the package definition in additional nested directory like
'.guix/modules/guile/package.scm' is not so bad and can replace
a '.guix/modules/guile-package.scm' variant.

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 12:45 Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH Nigko Yerden
  2024-08-13 14:38 ` pelzflorian (Florian Pelz)
@ 2024-08-14 14:00 ` Attila Lendvai
  2024-08-15 16:34   ` Nigko Yerden
  1 sibling, 1 reply; 30+ messages in thread
From: Attila Lendvai @ 2024-08-14 14:00 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: guix-devel

you might have stumbled on this issue:

(current-filename) is #f when guix pull'ing
https://issues.guix.gnu.org/55464

the discussion contains analysis and some things to try, and also a working solution that i'm currently using in my channel at:

https://github.com/attila-lendvai/guix-crypto

(see the `read-hashes-file` macro)

the extra twist is that it works fine while working on the code from e.g. emacs + geiser, but when you package it into a commit and try to `guix pull` the same code from the channel then it fails, and IIRC in some non-obvious way.

HTH,

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Him that I love, I wish to be free – even from me.”
	— Anne Morrow Lindbergh (1906–2001)



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-13 17:49       ` Nigko Yerden
@ 2024-08-14 15:35         ` pelzflorian (Florian Pelz)
  2024-08-14 16:40           ` pelzflorian (Florian Pelz)
  2024-08-14 19:29           ` pelzflorian (Florian Pelz)
  0 siblings, 2 replies; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-14 15:35 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

Nigko, that you attempted the recipe and found an error and a fix is
extremely helpful.  It seems to me that what is written in "The
Repository as a Channel", when copied literally, has never worked.

I tried with a guile package instead of your test-repo package,
using gnu-build-system rather than copy-build-system, and both
current guix master branch as well as old version-1.4.0 branch.
It still failed, like you wrote.

I turned your description of the work-around into a patch,
(see e-mail attachment), in your name and e-mail address, with
copyright.

Your work-around is to move the package, for which guix.scm is a
symlink, to its own directory.  It works for me.  And looks like a
good recommendation in general.

I believe Attila Lendvai’s bug with (current-filename) being #f
will *not* lead to a fix that makes this cookbook change unnecessary.

Is the patch okay?  Do you have suggestions for a better commit message?
May I push it to the guix repo?

Regards,
Florian


[-- Attachment #2: doc-cookbook-Fix-non-working-recipe.patch --]
[-- Type: text/x-patch, Size: 2666 bytes --]

From: Nigko Yerden <nigko.yerden@gmail.com>
Date: Wed, 14 Aug 2024 17:06:49 +0200
Subject: [PATCH] doc: cookbook: Fix non-working recipe.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes: <https://lists.gnu.org/archive/html/guix-devel/2024-08/msg00047.html>

* doc/guix-cookbook.texi (Software Development)[The Repository as a Channel]:
Move package to its own directory.  With Guix’ default GUILE_LOAD_PATH
setting, `local-file' cannot correctly follow symlinks otherwise.

Change-Id: I45d6878139e32f33b40465e272356c106a6d920a
---
 doc/guix-cookbook.texi | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/doc/guix-cookbook.texi b/doc/guix-cookbook.texi
index da67921ad0..bcb1bf67fc 100644
--- a/doc/guix-cookbook.texi
+++ b/doc/guix-cookbook.texi
@@ -25,6 +25,7 @@
 Copyright @copyright{} 2023-2024 Ludovic Courtès@*
 Copyright @copyright{} 2023 Thomas Ieong@*
 Copyright @copyright{} 2024 Florian Pelz@*
+Copyright @copyright{} 2024 Nigko Yerden@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -4623,9 +4624,9 @@ The Repository as a Channel
 symlink for the sake of @command{guix shell}:
 
 @lisp
-mkdir -p .guix/modules
-mv guix.scm .guix/modules/guile-package.scm
-ln -s .guix/modules/guile-package.scm guix.scm
+mkdir -p .guix/modules/guile
+mv guix.scm .guix/modules/guile/package.scm
+ln -s .guix/modules/guile/package.scm guix.scm
 @end lisp
 
 To make it usable as part of a channel, we need to turn our
@@ -4639,21 +4640,21 @@ The Repository as a Channel
 looks like this (not repeating things that haven't changed):
 
 @lisp
-(define-module (guile-package)
+(define-module (guile package)
   #:use-module (guix)
   #:use-module (guix git-download)   ;for ‘git-predicate’
   @dots{})
 
 (define vcs-file?
   ;; Return true if the given file is under version control.
-  (or (git-predicate (dirname (dirname (current-source-directory))))
+  (or (git-predicate (dirname (dirname (dirname (current-source-directory)))))
       (const #t)))                                ;not in a Git checkout
 
 (define-public guile
   (package
     (name "guile")
     (version "3.0.99-git")                          ;funky version number
-    (source (local-file "../.." "guile-checkout"
+    (source (local-file "../../.." "guile-checkout"
                         #:recursive? #t
                         #:select? vcs-file?))
     @dots{}))

base-commit: 174ecf5b1077d29498d9de22e27b13047f314feb
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-14 15:35         ` pelzflorian (Florian Pelz)
@ 2024-08-14 16:40           ` pelzflorian (Florian Pelz)
  2024-08-14 19:29           ` pelzflorian (Florian Pelz)
  1 sibling, 0 replies; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-14 16:40 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
> May I push it to the guix repo?

Or rather I will send it to guix-patches@gnu.org first.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-14 15:35         ` pelzflorian (Florian Pelz)
  2024-08-14 16:40           ` pelzflorian (Florian Pelz)
@ 2024-08-14 19:29           ` pelzflorian (Florian Pelz)
  2024-08-15  4:11             ` Nigko Yerden
  1 sibling, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-14 19:29 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
> Nigko, that you attempted the recipe and found an error and a fix is
> extremely helpful.  It seems to me that what is written in "The
> Repository as a Channel", when copied literally, has never worked.
>
> I tried with a guile package instead of your test-repo package,
> using gnu-build-system rather than copy-build-system, and both
> current guix master branch as well as old version-1.4.0 branch.
> It still failed, like you wrote.

Nonsense; it must have worked; 7.7 Wrapping Up lists

https://git.savannah.gnu.org/cgit/guile.git/tree/.guix/modules/guile-package.scm?id=cd57379b3df636198d8cd8e76c1bfbc523762e79

as proof.

I clearly did something wrong.  Sorry.

What’s the difference?  Will try to compare.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-14 19:29           ` pelzflorian (Florian Pelz)
@ 2024-08-15  4:11             ` Nigko Yerden
  2024-08-18 21:33               ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-15  4:11 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

Hello Florian,

pelzflorian (Florian Pelz) wrote:
> Nonsense; it must have worked; 7.7 Wrapping Up lists
> 
> https://git.savannah.gnu.org/cgit/guile.git/tree/.guix/modules/guile-package.scm?id=cd57379b3df636198d8cd8e76c1bfbc523762e79
> 
> as proof.
> 
> I clearly did something wrong.  Sorry.
> 
> What’s the difference?  Will try to compare.

For me pulling from this channel with subsequent

$ guix build guile@3.0.99-git

throws an error ("No such file or directory" "GUILE-VERSION"). However,

$ GUILE_LOAD_PATH= guix build guile@3.0.99-git

, which emulates system without [1] in Guile load path, works like a charm.
Thus, this repository behaves exactly as does the main branch of [2].

Perhaps many systems (e.g. Guix on foreign distributions) indeed does not
have [1] in Guile load path, and thus recipe from the Cookbook works for them.
    
Regards,
Nigko

[1] ~/.config/guix/current/share/guile/site/3.0/
[2] https://gitlab.com/anigko/test-channel.git


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-14 14:00 ` Attila Lendvai
@ 2024-08-15 16:34   ` Nigko Yerden
  0 siblings, 0 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-15 16:34 UTC (permalink / raw)
  To: Attila Lendvai; +Cc: guix-devel

Hello Attila Lendvai,

Thank you very much for your hints and references! Indeed I was puzzled by
weird behavior of 'current-filename'. I wrote another alternative
using '(module-filename (current-module))' based on
https://issues.guix.gnu.org/55464
See alt2 branch  of [1].

This variant does not need wrapping directory for the package file, and
has the same file structure as the main branch of [1]. The variable

(define package-dir
   (dirname
    (canonicalize-path
     (search-path %load-path
		 (module-filename (current-module))))))

is bounded to the current directory of 'test-repo-package.scm', so
the 'source' field of the package is

(local-file (string-append package-dir "/../..")
			"test-repo-checkout"
			#:recursive? #t
			#:select? vcs-file?))

It seems to work even without macro wrapper.

Regards,
Nigko

[1] https://gitlab.com/anigko/test-channel.git


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-15  4:11             ` Nigko Yerden
@ 2024-08-18 21:33               ` pelzflorian (Florian Pelz)
  2024-08-19  7:43                 ` Nigko Yerden
                                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-18 21:33 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

Hello Nigko and Attila.  My current belief is that any `local-file'
referring to outside the .guix-channel directory cannot work in channels
in all situations.  (Attila’s hashes files are inside guix-crypto’s
.guix-channel src directory.  They are fine.)

At first glance, Guix is faulty and does not respect all symlinks
in `absolute-dirname'.  The following diff should be applied to
guix.git, to resolve symlinks before computing dirname, not after:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: guix/utils.scm --]
[-- Type: text/x-patch, Size: 436 bytes --]

diff --git a/guix/utils.scm b/guix/utils.scm
index d8ce6ed886..322c9457e4 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -1124,7 +1124,7 @@ (define absolute-dirname
        ;; needs to be canonicalized.
        (if (string-prefix? "/" file)
            (dirname file)
-           (canonicalize-path (dirname file)))))))
+           (dirname (canonicalize-path file)))))))
 
 (define-syntax current-source-directory
   (lambda (s)


[-- Attachment #3: Type: text/plain, Size: 975 bytes --]



But this does not help with the cookbook’s guile channel nor Nigko’s
test-repo, even after repeatedly pulling a guix channel with this
change.

I might misunderstand .go compilation and whether macros are evaluated
at compile-time.  But maybe the error is because `local-file' is
relative to the current-source-directory from which the .go file was
compiled.  But the .go file was compiled from a directory union that
does not contain the source code, it only has the modules directory
listed in .guix-channel.  Only when the pre-compiled .go is not used,
the source directory can be found.  (Am I on the right track or
mistaken?)

/var/guix/profiles/per-user/florian/current-guix links to it.  guix gc
--referrers and --derivers show what is built from what.  Maybe I
misunderstand.  Maybe the channel build system would need changes.

In particular,

$ guix build -S guile@3.0.99-git

never works no matter the load path.

Regards,
Florian

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-18 21:33               ` pelzflorian (Florian Pelz)
@ 2024-08-19  7:43                 ` Nigko Yerden
  2024-08-19 19:28                   ` pelzflorian (Florian Pelz)
  2024-08-19  9:30                 ` Nigko Yerden
  2024-08-19 16:17                 ` Nigko Yerden
  2 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-19  7:43 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

Hello Florian!

Here is my version of your diff in attachment. With this patch
'guix build test-repo' works for the main branch of test-repo-channel,
i.e. it seems to solve one problem (I'm sure that 'guix build guile@3.0.99-git'
works as well for the guile channel, but didn't check this explicitly).
The other problem ('guix build -S test-repo') remains.

Regards,
Nigko

[-- Attachment #2: diff2 --]
[-- Type: text/plain, Size: 494 bytes --]

diff --git a/guix/utils.scm b/guix/utils.scm
index d8ce6ed886..8bd23097bf 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -1123,7 +1123,7 @@ (define absolute-dirname
        ;; If there are relative names in %LOAD-PATH, FILE can be relative and
        ;; needs to be canonicalized.
        (if (string-prefix? "/" file)
-           (dirname file)
+           (dirname (canonicalize-path file))
            (canonicalize-path (dirname file)))))))
 
 (define-syntax current-source-directory

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-18 21:33               ` pelzflorian (Florian Pelz)
  2024-08-19  7:43                 ` Nigko Yerden
@ 2024-08-19  9:30                 ` Nigko Yerden
  2024-08-19 16:17                 ` Nigko Yerden
  2 siblings, 0 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-19  9:30 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

On the second problem, I have found that while it is impossible
to obtain test-repo source code via 'guix build -S test-repo',
it can be retrieved programmatically from 'guix repl' REPL
using this commands:

(use-modules (test-repo-package)
	     (guix packages)
	     (guix gexp)
	     (guix store))
(define src (package-source test-repo))
(run-with-store (open-connection) (lower-object src))

So, perhaps this problem is not so thorough.

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-18 21:33               ` pelzflorian (Florian Pelz)
  2024-08-19  7:43                 ` Nigko Yerden
  2024-08-19  9:30                 ` Nigko Yerden
@ 2024-08-19 16:17                 ` Nigko Yerden
  2 siblings, 0 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-19 16:17 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

'guix build --debug=5 -S test-repo' build logs shows that the problem
sits in 'guix-build' procedure from 'guix/scripts/build.scm'. 'drv' local
variable is initialized by '(options->derivations store opts)' at line 762
to a list '("/gnu/store/...-test-repo-checkout") containing store
path to the true test-repo checkout (i.e. what is desired to be the output
of 'guix build -S'), and not a file with .drv extension or derivation object.
But the subsequent code in 'guix-build' (calls to 'show-derivation-outputs' and
'derivation->output-paths' procedures) supposes that 'drv' is a list of
derivation objects or .drv files, which leads to an error.

Regards,
Nigko








^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-19  7:43                 ` Nigko Yerden
@ 2024-08-19 19:28                   ` pelzflorian (Florian Pelz)
  2024-08-20  7:18                     ` Nigko Yerden
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-19 19:28 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Thank you, Nigko, for persevering.  It would be good if Guix’ claimed
features had no need for a GUILE_LOAD_PATH clearing work-around.  I had
not thought anymore that the source checkout can be referenced.

Could you send a patch about the `guix build' diff you debugged to
guix-patches@gnu.org?  Preferrably you would drop the whole
absolute-dirname’s `if' and canonicalize unconditionally, I guess.  Make
explicit in the docstring or in comments that symlinks are the reason.

`guix build -S' was just an example and a thought.  A fix would be nice,
especially now that you found the reason, but is nowhere near as
important.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-19 19:28                   ` pelzflorian (Florian Pelz)
@ 2024-08-20  7:18                     ` Nigko Yerden
  2024-08-20 16:49                       ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-20  7:18 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

pelzflorian (Florian Pelz) wrote:
> Could you send a patch about the `guix build' diff you debugged to
> guix-patches@gnu.org?  Preferrably you would drop the whole
> absolute-dirname’s `if' and canonicalize unconditionally, I guess.  Make
> explicit in the docstring or in comments that symlinks are the reason.
No. I view these patches of 'absolute-directory' as a demonstration that
the current behavior of 'local-file' is connected just with not following
symlinks, and not with something more complicated. The modification these
patches adds to 'current-source-directory' (so it would follow symlinks)
is worse than a hypothetical modification of 'local-file' because I suspect
it would break even more people's code.

Since cookbook's example refers to the Guile repo which does not work as a
channel in Guix with the default GUILE_LOAD_PATH, maybe first we need
to send a patch to Guile and then correct the cookbook accordingly?

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-20  7:18                     ` Nigko Yerden
@ 2024-08-20 16:49                       ` pelzflorian (Florian Pelz)
  2024-08-22  4:45                         ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-20 16:49 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Hello Nigko.  Your demonstration makes the guile channel work and is
valuable already, but we disagree which path to take now.

current-source-directory looks as if it were written without
consideration of symlinks; it resolves some and not others and none of
this is documented or tested.  My preferred option would be to fix it; I
doubt peoples’ configuration or channels rely on not resolving symlinks.

A bad option is to make `local-file' use some new
`current-source-directory-always-resolving-symlinks'.

Another bad option, to avoid any iota of ABI break in main guix, is to
change the guile repo, change the guix-cookbook, change the blog entry
from which the guix-cookbook section is derived.  To add a note along
the lines of “current-source-directory is a mess when using symlinks,
local-file uses it, so please use no local-file and use current-module
instead.”

I’m not involved with guix core, but IMO it would be wrong.  Rather
please send your `absolute-dirname' work to guix-patches, which the core
team would look at and have an opinion on.  Preferrably simplifying
`absolute-dirname' to not check (string-prefix? "/" file).

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-20 16:49                       ` pelzflorian (Florian Pelz)
@ 2024-08-22  4:45                         ` pelzflorian (Florian Pelz)
  2024-08-22  9:53                           ` Nigko Yerden
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-22  4:45 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Please accept my apology, Nigko.  Not for a second did I think about
Guix policy; we are meant to find a consensus.

Would you send a patch doing implementing another option to
guix-patches?  However, I would ask of you to send a patch to
guix-patches first and not to guile, since guile takes longer and
guix-patches would in my hope get others’ opinions involved.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-22  4:45                         ` pelzflorian (Florian Pelz)
@ 2024-08-22  9:53                           ` Nigko Yerden
  2024-08-22 13:22                             ` Nigko Yerden
  2024-08-22 16:00                             ` pelzflorian (Florian Pelz)
  0 siblings, 2 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-22  9:53 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

Hello Florian,

I am sure you have nothing to apologize for. I have been
thinking about your suggestion to submit `absolute-dirname' patch. In the
Guix manual at the end of the section 6.7 Creating a channel [1] there is the
following statement about Guix policy on changing API, not to mention ABI:

"We, Guix developers, never change APIs gratuitously, but we do not
commit to freezing APIs either. When you maintain package definitions
outside Guix, we consider that the compatibility burden is on you."

So it is okay to change the behavior of some procedures if it will improve
Guix. But does changing the 'current-source-directory' to follow symlinks
improve Guix? This change would adjust Guix in accordance with the
documentation... What? Sounds very wrong. Should be the other way round,
shouldn't it? Maybe this change would make Guix API more powerful and
convenient? I assume that generally there is a practical necessity in
determining current source directory both with and without following
symlinks, 50/50. If I need to follow symlinks I still may use
'current-source-directory' not following symlinks this way:

(dirname
  (canonicalize-path
   (string-append (current-source-directory) (current-filename))))

But what should I do if I need not to follow symlinks and
'current-source-directory' follows symlinks. There is no simple way.


pelzflorian (Florian Pelz) wrote:
> Would you send a patch doing implementing another option to
> guix-patches?  However, I would ask of you to send a patch to
> guix-patches first and not to guile, since guile takes longer and
> guix-patches would in my hope get others’ opinions involved.
Yes, I could. I wonder which alternative should I implement? One,
discussed earlier, adds an intermediate directory to the channel
relative path in the repository. This is simple, but somewhat
limited solution. Also changing directory structure may require
additional (and undesirable) modifications to other files in guile
repository.

Thanks to Attila there is another variant [2] which leaves the
directory structure intact:

(define checkout-dir
   ;; search %load-path for module filename,
   ;; follow symlink and return checkout directory
   (string-append (dirname (canonicalize-path
                            (search-path %load-path
                                         (module-filename (current-module)))))
                  "/../.."))

(define vcs-file?
   ;; Return true if the given file is under version control.
   (or (git-predicate checkout-dir)
       (const #t)))

(define-public guile
   (package
    ...
    (source (local-file checkout-dir "guile-checkout"
                        #:recursive? #t
                        #:select? vcs-file?))

What do you think?


Regards,
Nigko

[1] https://guix.gnu.org/en/manual/devel/en/html_node/Creating-a-Channel.html
[2] https://gitlab.com/anigko/test-channel/-/blob/ef07ce6904c16533f0fc21fda74216ce0a38bafd/.guix/modules/test-repo-package.scm
('channel-dir' is a macro instead of a variable, but I don't see any
real benefits in using macro here)


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-22  9:53                           ` Nigko Yerden
@ 2024-08-22 13:22                             ` Nigko Yerden
  2024-08-22 16:00                             ` pelzflorian (Florian Pelz)
  1 sibling, 0 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-22 13:22 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

Nigko Yerden wrote:
> (dirname
>   (canonicalize-path
>    (string-append (current-source-directory) (current-filename))))

Sorry, this expression for source directory is wrong. The correct one:

(dirname
  (canonicalize-path
   (string-append (current-source-directory) "/"
                  (basename (module-filename (current-module))))))

By the way this can also be used in the definition of 'checkout-dir', instead
of direct call to '(search-path %load-path ...)'.

Regards,
Nigko



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-22  9:53                           ` Nigko Yerden
  2024-08-22 13:22                             ` Nigko Yerden
@ 2024-08-22 16:00                             ` pelzflorian (Florian Pelz)
  2024-08-23  5:07                               ` Nigko Yerden
  1 sibling, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-22 16:00 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Hello Nigko.  I should have been looking for agreement from the start,
and now with your statement of the problem, I concur that without the
current implementation of `current-source-directory', it is hard not to
follow symlinks.  But then again, the current current-source-directory
already does follow symlinks in nearly all cases, even in configuration
files, except apparently when called in weird ways like local-file,
maybe because of Guix’ code staging.

It is not consistent, but actually I do not know why `local-file', when
calling `absolute-dirname', takes this case of `if'.


> Thanks to Attila there is another variant [2] which leaves the
> directory structure intact:

Documenting this work-around is not enough; we would have to also
document the rationale that local-file is weird and inconsistent,
because current-source-directory is weird and inconsistent.

I believe making local-file usable with any symlinks is better, and
changing absolute-dirname used by current-source-directory achieves
that.

The other alternative is making local-file not use
current-source-directory.


> ('channel-dir' is a macro instead of a variable, but I don't see any
> real benefits in using macro here)

Yes, macros are not necessary, because syntax-source, as used by
current-source-directory, is not necessary.  Why does
current-source-directory use syntax-source?  Perhaps it works even
outside modules.

Another reason against the other alternative.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-22 16:00                             ` pelzflorian (Florian Pelz)
@ 2024-08-23  5:07                               ` Nigko Yerden
  2024-08-23 15:47                                 ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-23  5:07 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

pelzflorian (Florian Pelz) wrote:
> But then again, the current current-source-directory
> already does follow symlinks in nearly all cases, even in configuration
> files
Don't see it neither in the code nor in our examples. 'syntax-source' doesn't
do this. The second branch of 'if' in 'absolute-dirname' bringing in
'canonicalize-path' is never executed in the examples (remember, the original
patch didn't work because of this).

> but actually I do not know why `local-file', when
> calling `absolute-dirname', takes this case of `if'.
Only the first branch of 'if' is executed in all practical cases I can imagine.

> Documenting this work-around is not enough; we would have to also
> document the rationale that local-file is weird and inconsistent,
Why do you think we should do this in this section of cookbook? Isn't it a separate
topic?

> because current-source-directory is weird and inconsistent
It isn't. It simply does not follow symlinks by design.

> Why does current-source-directory use syntax-source?
What can it use instead? Related syntaxes such as
'current-source-location' and 'current-filename' are all using
'syntax-source' under the hood.

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-23  5:07                               ` Nigko Yerden
@ 2024-08-23 15:47                                 ` pelzflorian (Florian Pelz)
  2024-08-23 16:25                                   ` pelzflorian (Florian Pelz)
  2024-08-24 14:47                                   ` Nigko Yerden
  0 siblings, 2 replies; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-23 15:47 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Hello Nigko.  I believe that it is natural to think local-file were the
right procedure to use, and it could be used with relative path.  Your
work-around puts in an absolute path, which perhaps really is what we
should put in the cookbook and guile-channel and blog post.

If a relative path is bad, we should warn against `local-file' with
relative paths in the manual and cookbook, and not just change the
example.  Or yet rather make a change to `local-file' so calling it with
"../.." relative paths is treated right in all cases.  Your diff makes
it right in more cases.

I believe `local-file' already is symlink-resolving in most cases:

Nigko Yerden <nigko.yerden@gmail.com> writes:
> pelzflorian (Florian Pelz) wrote:
>> But then again, the current current-source-directory
>> already does follow symlinks in nearly all cases, even in configuration
>> files
> Don't see it neither in the code nor in our examples. 'syntax-source' doesn't
> do this. The second branch of 'if' in 'absolute-dirname' bringing in
> 'canonicalize-path' is never executed in the examples (remember, the original
> patch didn't work because of this).
>
>> but actually I do not know why `local-file', when
>> calling `absolute-dirname', takes this case of `if'.
> Only the first branch of 'if' is executed in all practical cases I can
> imagine.

While processing guile-package.scm,

(search-path %load-path "guile-package.scm")

returns an absolute path if and only if guile-package.scm is in the
load-path, like when using it from a channel.  Then, your diff makes it
resolve symlinks.

If the configuration or package file is not in the load-path,
guile-package.scm is returned, absolute-dirname’s other `if' branch
calls `canonicalize-path' on all but the basename and directory symlinks
already got resolved.

Do I misunderstand?  I think symlinks are followed by design here.

(I wonder if such non-channel evaluation might cause problems when a
non-channel scheme file has the same name as a file in a channel.  This
might be a rationale for never using local-file with relative paths.)

In a guix repl evaluating a file at a path with symlinks, containing a
call to the built-in pk procedure on (current-source-directory),

(let ((f (open-input-file "/tmp/a/b/c/a/b/c/d.scm")))
     (eval (read f) (interaction-environment))
     (eval (read f) (interaction-environment))
     (eval (read f) (interaction-environment))
     (eval (read f) (interaction-environment)) )

all unnecessary path components remain, no symlinks are resolved and
absolute-dirname is not called at all.  This weird usage is unlike guix
home/system reconfigure or what normal people do.


>> Why does current-source-directory use syntax-source?
> What can it use instead? Related syntaxes such as
> 'current-source-location' and 'current-filename' are all using
> 'syntax-source' under the hood.

`current-module' from your work-around does not use syntax-source, but
outside modules cannot replace more powerful current-source-directory.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-23 15:47                                 ` pelzflorian (Florian Pelz)
@ 2024-08-23 16:25                                   ` pelzflorian (Florian Pelz)
  2024-08-24 14:47                                   ` Nigko Yerden
  1 sibling, 0 replies; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-23 16:25 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
> (I wonder if such non-channel evaluation might cause problems when a
> non-channel scheme file has the same name as a file in a channel.  This
> might be a rationale for never using local-file with relative paths.)

Indeed with weird load-paths

GUILE_LOAD_PATH=${GUILE_LOAD_PATH}:/home/florian/src/home-config/configs guix home reconfigure gnu.scm

;;; ("/gnu/store/nanvziq36krgh330yjhwpphcyfz5dyzm-guix-module-union/share/guile/site/3.0")

because syntax-source says filename is
"gnu.scm".

But not normally

GUILE_LOAD_PATH=${GUILE_LOAD_PATH} guix home reconfigure gnu.scm 

;;; ("/home/florian/src/home-config/configs")

because syntax-source says filename is
"/home/florian/src/home-config/configs/gnu.scm".

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-23 15:47                                 ` pelzflorian (Florian Pelz)
  2024-08-23 16:25                                   ` pelzflorian (Florian Pelz)
@ 2024-08-24 14:47                                   ` Nigko Yerden
  2024-08-26  8:50                                     ` pelzflorian (Florian Pelz)
  1 sibling, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-24 14:47 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

Hello Florian,

pelzflorian (Florian Pelz) wrote:
> While processing guile-package.scm,
> 
> (search-path %load-path "guile-package.scm")
> 
> returns an absolute path if and only if guile-package.scm is in the
> load-path, like when using it from a channel.  Then, your diff makes it
> resolve symlinks.
That's right.

> If the configuration or package file is not in the load-path,
> guile-package.scm is returned, absolute-dirname’s other `if' branch
> calls `canonicalize-path' on all but the basename and directory symlinks
> already got resolved.
No, in this case 'search-path' returns #f. But if 'syntax-source' gives
absolute path, then 'current-source-directory' returns 'dirname' of this
path without calling 'absolute-dirname'.

> Indeed with weird load-paths
> 
> GUILE_LOAD_PATH=${GUILE_LOAD_PATH}:/home/florian/src/home-config/configs guix home reconfigure gnu.scm
> 
> ;;; ("/gnu/store/nanvziq36krgh330yjhwpphcyfz5dyzm-guix-module-union/share/guile/site/3.0")
> 
> because syntax-source says filename is
> "gnu.scm".
> 
> But not normally
> 
> GUILE_LOAD_PATH=${GUILE_LOAD_PATH} guix home reconfigure gnu.scm 
> 
> ;;; ("/home/florian/src/home-config/configs")
> 
> because syntax-source says filename is
> "/home/florian/src/home-config/configs/gnu.scm".
This observation shows that 'current-source-directory' is really bad because
'syntax-source' is bad. Sometimes 'syntax-source' gives an absolute path, and
sometimes a relative one, and I don't understand under what conditions.
And '(module-filename (current-module))' does not seem to be better.
In my experiments it gives exactly the same filename as 'syntax-source'.
But this bad behavior of 'current-source-directory' is unrelated to symlinks and
'if' condition in 'absolute-dirname'. All Guile load paths I have seen before are
absolute paths, and the second branch of 'if' is not executed for them.

Maybe we should leave 'absolute-dirname' alone, as well as 'current-source-directory',
and consider 'local-file' more closely.

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-24 14:47                                   ` Nigko Yerden
@ 2024-08-26  8:50                                     ` pelzflorian (Florian Pelz)
  2024-08-28 12:36                                       ` Nigko Yerden
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-26  8:50 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Hello Nigko.

Nigko Yerden <nigko.yerden@gmail.com> writes:
> pelzflorian (Florian Pelz) wrote:
>> If the configuration or package file is not in the load-path,
>> guile-package.scm is returned, absolute-dirname’s other `if' branch
>> calls `canonicalize-path' on all but the basename and directory symlinks
>> already got resolved.
> No, in this case 'search-path' returns #f. But if 'syntax-source' gives
> absolute path, then 'current-source-directory' returns 'dirname' of this
> path without calling 'absolute-dirname'.

Oops.  Indeed, your view is right.

Still, when would your diff break someone else’s code?

They would need to currently use `local-file' or `current-source-directory'
to reference a file in .config/guix/current/share/guile/site/3.0,
which your diff would break.  But this still seems unlikely to me.

What `local-file' needs is a kind of current-source-directory.
Current `current-source-directory' does a bad job in channels.  Your
diff does fix this.  The compatibility burden is on the channel
authors.  `current-source-directory' would no longer be bad in what we
deem realistic use.

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-26  8:50                                     ` pelzflorian (Florian Pelz)
@ 2024-08-28 12:36                                       ` Nigko Yerden
  2024-08-28 16:40                                         ` pelzflorian (Florian Pelz)
  0 siblings, 1 reply; 30+ messages in thread
From: Nigko Yerden @ 2024-08-28 12:36 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

Hello Florian,

pelzflorian (Florian Pelz) wrote:
> Still, when would your diff break someone else’s code?
Instead of answering this tough question I decided to make another
patch (see attachment) which is guaranteed not to break another's code:

- The patch adds 'follow-symlinks?' argument  to 'current-source-directory'
   macro and 'absolute-dirname' procedure.
- In case of 'current-source-directory' the argument is optional and defaults
   to #f. Thus new API is backward compatible with the old one.
- In case of 'absolute-dirname' the argument is mandatory, but this procedure
   is internal to (guix utils) module and therefore the modification does not
   change public API. The usage of 'absolute-dirname' inside this module
   (only two occurrences) is corrected accordingly.
- 'local-file' macro from (guix gexp) uses 'current-source-directory' with
   follow-symlinks? argument set to #t.

Regards,
Nigko

[-- Attachment #2: diff3 --]
[-- Type: text/plain, Size: 3790 bytes --]

diff --git a/guix/gexp.scm b/guix/gexp.scm
index 74b4c49f90..5911ca4815 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -508,7 +508,7 @@ (define-syntax local-file
        (string? (syntax->datum #'file))
        ;; FILE is a literal, so resolve it relative to the source directory.
        #'(%local-file file
-                      (delay (absolute-file-name file (current-source-directory)))
+                      (delay (absolute-file-name file (current-source-directory #t)))
                       rest ...))
       ((_ (assume-valid-file-name file) rest ...)
        ;; FILE is not a literal, so resolve it relative to the current
diff --git a/guix/utils.scm b/guix/utils.scm
index d8ce6ed886..b5fcf8cb28 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -1110,41 +1110,47 @@ (define (canonical-newline-port port)
 
 (define (%guix-source-root-directory)
   "Return the source root directory of the Guix found in %load-path."
-  (dirname (absolute-dirname "guix/packages.scm")))
+  (dirname (absolute-dirname "guix/packages.scm" #f)))
 
 (define absolute-dirname
   ;; Memoize to avoid repeated 'stat' storms from 'search-path'.
-  (mlambda (file)
+  (mlambda (file follow-symlinks?)
     "Return the absolute name of the directory containing FILE, or #f upon
-failure."
+failure. Follow symlinks if FOLLOW-SYMLINKS? is true."
     (match (search-path %load-path file)
       (#f #f)
       ((? string? file)
-       ;; If there are relative names in %LOAD-PATH, FILE can be relative and
-       ;; needs to be canonicalized.
-       (if (string-prefix? "/" file)
-           (dirname file)
-           (canonicalize-path (dirname file)))))))
+       (if follow-symlinks?
+	   (dirname (canonicalize-path file))
+	   ;; If there are relative names in %LOAD-PATH, FILE can be relative
+	   ;; and needs to be canonicalized.
+	   (if (string-prefix? "/" file)
+               (dirname file)
+               (canonicalize-path (dirname file))))))))
 
 (define-syntax current-source-directory
   (lambda (s)
     "Return the absolute name of the current directory, or #f if it could not
-be determined."
+be determined. Do not follow symlinks if FOLLOW-SYMLINKS? is false (the default)."
+    (define (source-directory follow-symlinks?)
+      (match (assq 'filename (or (syntax-source s) '()))
+	(('filename . (? string? file-name))
+	 ;; If %FILE-PORT-NAME-CANONICALIZATION is 'relative, then FILE-NAME
+	 ;; can be relative.  In that case, we try to find out at run time
+	 ;; the absolute file name by looking at %LOAD-PATH; doing this at
+	 ;; run time rather than expansion time is necessary to allow files
+	 ;; to be moved on the file system.
+	 (if (string-prefix? "/" file-name)
+	     (dirname (if follow-symlinks?
+			  (canonicalize-path file-name)
+			  file-name))
+	     #`(absolute-dirname #,file-name #,follow-symlinks?)))
+	((or ('filename . #f) #f)
+	 ;; raising an error would upset Geiser users
+	 #f)))
     (syntax-case s ()
-      ((_)
-       (match (assq 'filename (or (syntax-source s) '()))
-         (('filename . (? string? file-name))
-          ;; If %FILE-PORT-NAME-CANONICALIZATION is 'relative, then FILE-NAME
-          ;; can be relative.  In that case, we try to find out at run time
-          ;; the absolute file name by looking at %LOAD-PATH; doing this at
-          ;; run time rather than expansion time is necessary to allow files
-          ;; to be moved on the file system.
-          (if (string-prefix? "/" file-name)
-              (dirname file-name)
-              #`(absolute-dirname #,file-name)))
-         ((or ('filename . #f) #f)
-          ;; raising an error would upset Geiser users
-          #f))))))
+      ((_) (source-directory #f))
+      ((_ follow-symlinks?) (source-directory #'follow-symlinks?)))))
 
 \f
 ;;;

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-28 12:36                                       ` Nigko Yerden
@ 2024-08-28 16:40                                         ` pelzflorian (Florian Pelz)
  2024-08-29  6:17                                           ` Nigko Yerden
  0 siblings, 1 reply; 30+ messages in thread
From: pelzflorian (Florian Pelz) @ 2024-08-28 16:40 UTC (permalink / raw)
  To: Nigko Yerden; +Cc: Attila Lendvai, guix-devel

Hello Nigko.

Nigko Yerden <nigko.yerden@gmail.com> writes:
> pelzflorian (Florian Pelz) wrote:
>> Still, when would your diff break someone else’s code?
> Instead of answering this tough question I decided to make another
> patch (see attachment) which is guaranteed not to break another's code:

In my opinion, when you send this diff to guix-patches (with your notes
what it does like in this e-mail; the notes maybe will become part of
the commit message), you should include the rationale that you made the
macro complicated *because* you think it is possible that someone’s code
depends on it.  Like you wrote here.

I doubt it and would prefer the old, simple version, but I tend to not
consider all cases.  I trust the team for Guix core.

Could you send this diff to guix-patches@gnu.org?

Regards,
Florian


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH
  2024-08-28 16:40                                         ` pelzflorian (Florian Pelz)
@ 2024-08-29  6:17                                           ` Nigko Yerden
  0 siblings, 0 replies; 30+ messages in thread
From: Nigko Yerden @ 2024-08-29  6:17 UTC (permalink / raw)
  To: pelzflorian (Florian Pelz); +Cc: Attila Lendvai, guix-devel

pelzflorian (Florian Pelz) wrote:
> Could you send this diff to guix-patches@gnu.org?
Here it is: https://issues.guix.gnu.org/72867

Regards,
Nigko


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2024-08-29  6:17 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-13 12:45 Cookbook recipe from "The Repository as a Channel" section does not work for Guix with properly configured GUILE_LOAD_PATH Nigko Yerden
2024-08-13 14:38 ` pelzflorian (Florian Pelz)
2024-08-13 15:06   ` Nigko Yerden
2024-08-13 17:25     ` pelzflorian (Florian Pelz)
2024-08-13 17:49       ` Nigko Yerden
2024-08-14 15:35         ` pelzflorian (Florian Pelz)
2024-08-14 16:40           ` pelzflorian (Florian Pelz)
2024-08-14 19:29           ` pelzflorian (Florian Pelz)
2024-08-15  4:11             ` Nigko Yerden
2024-08-18 21:33               ` pelzflorian (Florian Pelz)
2024-08-19  7:43                 ` Nigko Yerden
2024-08-19 19:28                   ` pelzflorian (Florian Pelz)
2024-08-20  7:18                     ` Nigko Yerden
2024-08-20 16:49                       ` pelzflorian (Florian Pelz)
2024-08-22  4:45                         ` pelzflorian (Florian Pelz)
2024-08-22  9:53                           ` Nigko Yerden
2024-08-22 13:22                             ` Nigko Yerden
2024-08-22 16:00                             ` pelzflorian (Florian Pelz)
2024-08-23  5:07                               ` Nigko Yerden
2024-08-23 15:47                                 ` pelzflorian (Florian Pelz)
2024-08-23 16:25                                   ` pelzflorian (Florian Pelz)
2024-08-24 14:47                                   ` Nigko Yerden
2024-08-26  8:50                                     ` pelzflorian (Florian Pelz)
2024-08-28 12:36                                       ` Nigko Yerden
2024-08-28 16:40                                         ` pelzflorian (Florian Pelz)
2024-08-29  6:17                                           ` Nigko Yerden
2024-08-19  9:30                 ` Nigko Yerden
2024-08-19 16:17                 ` Nigko Yerden
2024-08-14 14:00 ` Attila Lendvai
2024-08-15 16:34   ` Nigko Yerden

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.