Re: [Xen-devel] [PATCH RFC] firmware/shim: fix Xen tree setup

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 31, 2018 at 09:55:21AM -0700, Jan Beulich wrote:
> There are multiple issues here, and I'm happy to split the patch up if
> that's what it takes:
> - "set -e" on a separate Makefile line is meaningless. Glue together all
>   the lines that this is supposed to cover.
> - I have no idea what *.d1 is supposed to refer to - we only have .*.d
>   and .*.d2 files (not also the leading dot).

These two are genuine bugs, thanks for taking the time to look into
them.

> - I have also no idea what *.1 is meant to cover. Instead also exclude
>   preprocessed and non-source assembly files.

A copy-n-paste from the mini-os link farm rune. Feel free to remove it.

> - Excluding symlinks in the source tree is a problem for me: Short of
>   out-of-tree builds, in order to easily build test multiple
>   configurations, I'm setting up my build trees as trees of symlinks
>   into the source tree. Hence the original logic would find only the few
>   generated files under config/. I do realize though that find's -xtype
>   primary is a non-standard extension (explicitly having "! type -l"
>   seems pointless though, at least for standard conforming find, as
>   "-type f" is supposed to only find non-symlinked files).
> 

At the time I thought whatever symlinks we have in tree should be
created by xen's build system itself hence there was no need to copy
them around, and it would have to avoid latent problem like linking to
wrong files etc.

I think you have valid usecase for symlinks, so I'm fine with the change
you make.

> Irrespective of the changes I'm still observing "mkdir -p" to report a
> missing operand, as config/ has no subdirs. Oddly enough this doesn't
> cause the whole command (and hence the build to fail), despite the
> "set -e" now covering the entire set of commands - perhaps a quirk of
> the relatively old bash I've seen this with (a few simple experiments
> suggest that commands inside () producing a non-success status would
> exit the inner shell, but not the outer one).
> 

I did see error when I wrote this bit hence I specifically hacked the
rune to preserve "." in the output. We can add that back if necessary.

I suppose you will send out a new version at some point?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel