Re: [PATCH] Re: tangle option to not write a file with same contents?

[Ihor Radchenko <yantar92@gmail.com>]

Max Nikulin <manikulin@gmail.com> writes:

> On 30/05/2022 10:14, Ihor Radchenko wrote:
>> I applied the discussed two patches onto main via 1525a5a64 and
>> f6f26d4ce. The suggested amendments were incorporated.
>
> So Greg's feature request is implemented and it is great.
>
> I am less confident with `org-babel-load-file' though. Due to poor error 
> handling in `org-babel-tangle' & Co., I am unsure that I can provide an 
> example when new code incorrectly updates file modification time despite 
> tangle failure, but I do not like that modification time is changed 
> before attempt to tangle. Generally I expected that it is performed 
> after tangling, perhaps with check that `org-babel-tangle-file' returned 
> expected file name (as some hope for success).

I agree that updating the modification time after tangling will be
slightly more robust in common cases. Though not necessarily strictly
better.

Handling errors is challenging. org-babel-tangle potentially writes to
multiple files in unspecified order. Tangle error may happen in the
middle of tangling when some files are written and some are not. Those
tangled files could also depend on each other during loading (think of
config.org tangling into init.el and config.el one requiring another).
Then, an error during tangling can result in some files being tangled to
newer versions and some still having old versions.

Now, consider that config.org is tangled to config.el and then to
init.el with the latter tangling process failing. Next time we call
org-babel-load-file, the updated config.el will be loaded despite error
and incorrectly require the old init.el.

Another case is when init.el is tangled first and config.el tangling
fails. This time, it matters whether we update the modification time of
config.el (before of after tangling). In our current code (with my
patch), the older version of config.el will be touched despite tangling
error and next time old config.el + new init.el will be loaded.

The existing code will create problems in both the described cases.
If we update file modification time only after tangling succeeds without
error, only the first case will be problematic.

So, your proposal will indeed be more robust in a sense that it will
prevent _some_ problems when tangling fails.

However, I see the "robustness" as a bad thing here. org-babel-load will
sometimes fail and sometimes not depending on the where in config.org
the error appears. I see it as a potential nightmare for debugging.

Ideally, we should solve the problem with tangle errors in some other
way. I am not sure how (or rather haven't spent much time thinking).

Ideas are welcome!

> I have realized that the case was not exactly the same when Nicolas 
> asked for a patch correction:
>
> Nicolas Goaziou to emacs-orgmode. Re: [PATCH] Fix moving cursor in 
> org-set-tags-command. Fri, 08 May 2020 09:53:55 +0200.
> https://list.orgmode.org/87ftca6ewc.fsf@nicolasgoaziou.fr
>
> "Please replace `and' with `when' if side-effects are involved."

I see. `and' is indeed considered a logical statement and should not
normally be abused for side-effects. But it does not imply the opposite
for `when'.

>> Everything is tangled to files prior to running the benchmark. This way,
>> none of the tangled files should change when calling org-babel-tangle
>> and no disk caches should be involved.
>
> I am trying to say that in your benchmark file are not read from disk, 
> almost certainly they are cached in memory. Besides write cache, while 
> reading, content may be taken from cache as well.
>
> Actually I am just speculating, while you have benchmark result. It is 
> unlikely that I will decide to spend some time arranging another test to 
> convince you with numbers, priority of such activity is not high enough.

Let me clarify. I am looking at this benchmark from a broader
perspective:
- We want the patch to improve performance.
- We don't want the patch to degrade performance.

1. My benchmark certainly shows that there is an improvement in some
   cases. It is something we want.

2. My benchmark may not be accurate depending on the system state. It is
   also fine. Just shows that improvement might not happen in 100% of
   cases.

3. Is my benchmark degrading performance? I do not see how and I do not
   see how your observations prove otherwise. So, unless you can see how
   my patch degrades performance on some systems, I do not see any
   reason to perform more accurate benchmarking other than for science.

Best,
Ihor