Dependencies should include "realpath"
Since this issue just came up related to my MacPorts port: The list of dependencies for notmuch should include "realpath". configure [1] relies on realpath to generate the value for rsti_dir. [1] https://git.notmuchmail.org/git?p=notmuch;a=blob;f=configure;h=40e8b2559e86b40985ef2c6dc142eb0104c3ddb6;hb=45193bab16c728ba892a5d45fc62ef59e2a6ef85#l1539 While Linux usually comes with realpath included, macOS does not, and I am not quite sure about BSD variants. -Ralph ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
Ralph Seichter writes: > Since this issue just came up related to my MacPorts port: The list of > dependencies for notmuch should include "realpath". configure [1] relies > on realpath to generate the value for rsti_dir. > Do you have a suggested replacement? I guess some inline perl with "use Cwd 'realpath'" would probably work, although I haven't tested it. ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
* David Bremner: > Do you have a suggested replacement? I guess some inline perl with "use > Cwd 'realpath'" would probably work, although I haven't tested it. At a quick glance, that particular section of "configure" is run by doc/conf.py to generate three lines of Python code and store the result as sphinx.config, correct? If so, my preferred choice would be to use Python to figure out the absolute path, e.g. like so: rsti_dir = os.path.abspath('emacs') This shows the generated result, and I assume that emacs is a directory in the source tree? I also wonder if an absolute directory path is really required for the doc-build to work. The segment of conf.py which uses the generated config file does not look convincing to me anyway. Apparently the original author did not like it either, which is why the segment is labelled as "hacky". It should probably be overhauled, and not only because it uses the statement open(rsti_dir+'/'+file) which will potentially fail, depending on the build platform. -Ralph ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
Ralph Seichter writes: > * David Bremner: > >> Do you have a suggested replacement? I guess some inline perl with "use >> Cwd 'realpath'" would probably work, although I haven't tested it. > > At a quick glance, that particular section of "configure" is run by > doc/conf.py to generate three lines of Python code and store the result > as sphinx.config, correct? If so, my preferred choice would be to use > Python to figure out the absolute path, e.g. like so: > > rsti_dir = os.path.abspath('emacs') > > This shows the generated result, and I assume that emacs is a directory > in the source tree? I also wonder if an absolute directory path is really > required for the doc-build to work. Someone (TM) would have to check that this version did not break out-of-tree builds. Other than that it seems plausible. > The segment of conf.py which uses the generated config file does not > look convincing to me anyway. Apparently the original author did not > like it either, which is why the segment is labelled as "hacky". It > should probably be overhauled, and not only because it uses the > statement open(rsti_dir+'/'+file) which will potentially fail, depending > on the build platform. It's somewhat orthogonal to this discussion, but I don't object to a patch replacing the hardcoded '/' with the appropriate python path manipulation code. I think windows compatibility for notmuch has much bigger issues, but it doesn't hurt to clean up the code. Other than that, I'm not sure what an "overhaul" would involve. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
* David Bremner: > I'm not sure what an "overhaul" would involve. Looking at conf.py, I find the following confusing: lines = ['.. include:: /../emacs/rstdoc.rsti\n\n'] # in the source tree for file in ('notmuch.rsti', 'notmuch-lib.rsti', 'notmuch-show.rsti', 'notmuch-tag.rsti'): lines.extend(open(rsti_dir+'/'+file)) rst_epilog = ''.join(lines) del lines "lines" is of type List[str]. In a loop which uses "file" (a reserved expression), open() returns file handles on success. The string (!) list is then extended with file handles, and after the loop, members of the list (one string, n file handles) are concatenated using an empty string. What is the reasoning behind this code segment? As for generating a config file with configure and then reading and executing individual lines from Python: Why not write to doc/dyncf.py and use "from .dyncf import tags, rsti_dir" in conf.py? Maybe I am off on a tangent, though. It would help me if I knew what problem you were actually trying to solve?` -Ralph ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
On Sat, Oct 17 2020, Ralph Seichter wrote: > * David Bremner: > >> Do you have a suggested replacement? I guess some inline perl with "use >> Cwd 'realpath'" would probably work, although I haven't tested it. > > At a quick glance, that particular section of "configure" is run by > doc/conf.py to generate three lines of Python code and store the result > as sphinx.config, correct? If so, my preferred choice would be to use > Python to figure out the absolute path, e.g. like so: > > rsti_dir = os.path.abspath('emacs') Good suggestion, anyway, the simplest change would be just: -printf "rsti_dir = '%s'\n" $(realpath emacs) +printf "rsti_dir = '%s'\n" $(cd emacs && pwd -P) > This shows the generated result, and I assume that emacs is a directory > in the source tree? I also wonder if an absolute directory path is really > required for the doc-build to work. Absolute path is safer when doing out-of-tree builds > The segment of conf.py which uses the generated config file does not > look convincing to me anyway. Apparently the original author did not > like it either, which is why the segment is labelled as "hacky". It > should probably be overhauled, and not only because it uses the > statement open(rsti_dir+'/'+file) which will potentially fail, depending > on the build platform. Sure, the doc/conf.py is somewhat hacky, but has done the work for couple of years already :D (not that anyone who would like to make is better would not be welcome to do so). Then, just for the record, I think open(rsti_dir+'/'+file) is fine, and I don't see it failing on any imaginable system notmuch work (now?;) -- I am even personally changing some os.path.join(...) commands to use that concatenation instead, just to reduce complexity and line count elsewhere... Tomi > > -Ralph ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
* Tomi Ollila: >> rsti_dir = os.path.abspath('emacs') > > Good suggestion, anyway, the simplest change would be just: > > - printf "rsti_dir = '%s'\n" $(realpath emacs) > + printf "rsti_dir = '%s'\n" $(cd emacs && pwd -P) Looks like "pwd -P" is part of the Open Group base spec for pwd, so it should be available on supported platforms. > Then, just for the record, I think open(rsti_dir+'/'+file) is fine, > and I don't see it failing on any imaginable system notmuch work > (now?;) As they say, you have the right to your opinion. ;-) I don't think it is "fine" at all. While rsti_dir+'/'+file may work, depending on the platform used, os.path.join(rsti_dir, file) *will* work. Quoting Python's "os" description, first sentence: "This module provides a portable way of using operating system dependent functionality." I see no viable reason not to use "os". > I am even personally changing some os.path.join(...) commands to use > that concatenation instead, just to reduce complexity and line count > elsewhere... I am sorry to hear that. There is nothing complex about using os.path, and as far as I am concerned, correctness and ease of maintenance beats a reduced line count every single time. -Ralph ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: Dependencies should include "realpath"
On Tue, Oct 20 2020, Ralph Seichter wrote: > * Tomi Ollila: > >>> rsti_dir = os.path.abspath('emacs') >> >> Good suggestion, anyway, the simplest change would be just: >> >> - printf "rsti_dir = '%s'\n" $(realpath emacs) >> + printf "rsti_dir = '%s'\n" $(cd emacs && pwd -P) > > Looks like "pwd -P" is part of the Open Group base spec for pwd, so it > should be available on supported platforms. oh, you're right, I remembered configure was bash(1) script, but, it has #!/bin/sh -- (although "traditional" bourne shell would not do it) Thanks for checking! (dropped our conversation about use of os.path... :D) > > -Ralph Tomi ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org