lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Concinnity check and hardcoded paths


From: Greg Chicares
Subject: Re: [lmi] Concinnity check and hardcoded paths
Date: Tue, 13 Oct 2020 01:11:44 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0

On 2020-10-13 00:14, Vadim Zeitlin wrote:
>  Hello,
> 
>  I'm probably going to apply the changes at
> 
>       https://github.com/let-me-illustrate/lmi/pull/164/files
> 
> soon, in order to check concinnity automatically for all PRs, but it
> somewhat bothers me that we need to create files with hardcoded names, such
> as /tmp/schroot_env (but also /opt/lmi/src/lmi), in order to allow this
> test to pass.

The widespread assumption that 'prefix="/opt/lmi"' is probably harder
to alleviate (although I think we may be getting close), so for now
let's consider /tmp/schroot_env . My first thought was to disable
SC1091, as I thought I had done in some other instances, but what I
was remembering was the similar SC1090:

  # shellcheck disable=SC1090
  . "$srcdir"/set_toolchain.sh

Anyway, I think I should make this change everywhere:

  . ./lmi_setup_inc.sh
+ # shellcheck disable=SC1091
  . /tmp/schroot_env

>  So I wonder if you would be open to an idea of working around this problem
> in some other way? The problem is that the only one I see right now is to
> explicitly disable SC1091 before each inclusion of this file, but this
> would require many changes and doesn't seem particularly appealing. OTOH it
> is "correct", for some meaning of "correct": we really don't care about
> checking this file for the errors.

I think it's not just "correct", but *correct*. This file is just an
artifact of the lmi_setup* scripts, and otherwise needn't exist.
Without 'shellcheck disable=SC1091', it seems that a fresh checkout
of lmi wouldn't pass the concinnity test, so this would resolve an
actual defect.

>  In any case, there are other possibilities, e.g. we could have some kind
> of restrained concinnity check that wouldn't consider lmi_setup*.sh at all,
> as these files are only ever modified by you anyhow, so we don't risk
> breaking anything in them.

Build systems are difficult to modify correctly, especially in a
chroot "hall of mirrors". CI is a valuable safeguard against the
risk that I'll make a mistkae.

>  But I don't want to spend time on this if you think that it's not really a
> problem in the first place, hence why I'm asking this question first. Of

I can sometimes write a few lines of C++ code without any error.
That's never the case with shell code: I really need shellcheck.
Having decided to use a lint-like tool, one must accept that
'# lint:ignore' annotations will be necessary. I'm at peace
with that.

And I don't see any good alternative to /tmp/schroot_env .
This mailing list's archives will show the alternatives I
explored, all of which were worse.

> course, it's not a secret that I hope that one day lmi could be compiled
> entirely inside its own part of file system (specified by "prefix" value,
> which already exists, but currently has to be set to /opt/lmi for things to
> work) and for this there shouldn't be any hardcoded absolute paths anywhere

I'm not ready to grapple with that today, but I don't see
any reason why it shouldn't be a tractable problem, now
that so much of the usual work of building is scripted.

> in it, so I also hope for a positive answer, but I realize that this isn't
> the most crucial or urgent thing to do, so, again, I can perfectly well
> commit the current changes and at least achieve the main goal, which is
> worth pursuing, i.e. ensure that all checks are done automatically for all
> changes. Please let me know what do you think.

It's an actual defect, because a fresh checkout of lmi fails
the concinnity test. Therefore, I should fix that defect,
and then you'll have no need to work around it. Sound okay?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]