Re: Newt feature: run custom commands at build time

[Christopher Collins]

> I do not have a strong opinion on this, we can keep it as is, however... I
> expected that these paths are relative to package root but seems like they
> are relative to project root. Is this intended behavior? I did not find any
> way to address script in a package other that using full path, i.e.
> 'repos/.../.../script.sh' which is counter-intuitive tbh. Perhaps I
> misunderstood description, but my understanding was that cwd is set to
> project root, but newt will still look for a script in package root - this
> would make more sense I think.

Ah, I see what you mean.  You're right - the script path should be
relative to the package directory, not to the project.  I will change
this.

The user may also want to run scripts relative to the project root.
There is an environment variable containing the project root
(`MYNEWT_PROJECT_ROOT`).  This variable is not set in the newt process
itself, but I think it would be useful.  I will change newt so that it
defines this setting in its own process.

Thanks,
Chris

Re: Newt feature: run custom commands at build time

[Andrzej Kaczmarek]

Hi Chris,

On Tue, Oct 1, 2019 at 3:21 AM Christopher Collins  wrote:

> Hi Andrzej,
>
> On Thu, Sep 26, 2019 at 07:24:54PM +0200, Andrzej Kaczmarek wrote:
> > This looks very good! I was thinking if it would be possible to reference
> > other targets (i.e. artifacts) from scripts but with the latest addition
> of
> > shared folder this does not seem to be a problem since it can be also
> > shared with another newt build invoked from script and we can copy/write
> > data there. I did not yet check how this work in practice but will give
> it
> > a try and perhaps then I'll have some extra ideas.
>
> Thanks for taking a look!
>
> > > post_cmds (run after the build).
> > >
> > > ### EXAMPLE
> > >
> > > Example (apps/blinky/pkg.yml):
> > >
> > > pkg.pre_cmds:
> > > scripts/pre_build1.sh: 100
> > > scripts/pre_build2.sh: 200
> > >
> > > pkg.post_cmds:
> > > scripts/post_build.sh: 100
> > >
> >
> > I assume these are relative to package root so perhaps we could assume
> > there is scripts/ subdir and execute from there by default? Just the same
> > as we have src/ and include/.
>
> I'm reluctant to use an implicit path here.  I think it is good to be
> explicit so that there is no confusion about where a script is located.
>
> We use an implicit "targets" path, but I feel like that is easier to
> justify because it saves the user from constantly typing the same thing.
> I don't think this custom command feature will be used very often at
> all, so I am not sure an implicit path would add much in the way of
> convenience.
>
> It's easy to add an implicit path later, but impossible to remove it.
> Unless you have a strong opinion on this, I suggest we give the feature
> some time without the implicit path and make the decision later.
>

I do not have a strong opinion on this, we can keep it as is, however... I
expected that these paths are relative to package root but seems like they
are relative to project root. Is this intended behavior? I did not find any
way to address script in a package other that using full path, i.e.
'repos/.../.../script.sh' which is counter-intuitive tbh. Perhaps I
misunderstood description, but my understanding was that cwd is set to
project root, but newt will still look for a script in package root - this
would make more sense I think.


> Thanks,
> Chris
>

Best,
Andrzej

Re: Newt feature: run custom commands at build time

[Christopher Collins]

Hi Andrzej,

On Thu, Sep 26, 2019 at 07:24:54PM +0200, Andrzej Kaczmarek wrote:
> This looks very good! I was thinking if it would be possible to reference
> other targets (i.e. artifacts) from scripts but with the latest addition of
> shared folder this does not seem to be a problem since it can be also
> shared with another newt build invoked from script and we can copy/write
> data there. I did not yet check how this work in practice but will give it
> a try and perhaps then I'll have some extra ideas.

Thanks for taking a look!

> > post_cmds (run after the build).
> >
> > ### EXAMPLE
> >
> > Example (apps/blinky/pkg.yml):
> >
> > pkg.pre_cmds:
> > scripts/pre_build1.sh: 100
> > scripts/pre_build2.sh: 200
> >
> > pkg.post_cmds:
> > scripts/post_build.sh: 100
> >
> 
> I assume these are relative to package root so perhaps we could assume
> there is scripts/ subdir and execute from there by default? Just the same
> as we have src/ and include/.

I'm reluctant to use an implicit path here.  I think it is good to be
explicit so that there is no confusion about where a script is located.

We use an implicit "targets" path, but I feel like that is easier to
justify because it saves the user from constantly typing the same thing.
I don't think this custom command feature will be used very often at
all, so I am not sure an implicit path would add much in the way of
convenience.

It's easy to add an implicit path later, but impossible to remove it.
Unless you have a strong opinion on this, I suggest we give the feature
some time without the implicit path and make the decision later.

Thanks,
Chris

Re: Newt feature: run custom commands at build time

[Andrzej Kaczmarek]

Hi Chris,

On Tue, Sep 24, 2019 at 2:48 AM Christopher Collins 
wrote:

> Hello all,
>
> I have implemented a feature in newt: the ability to run custom shell
> commands at build time.
>
> 
>
> Is there any extra functionality that you'd like to see here?  All
> comments are appreciated.  I am duplicating the PR text here, but the
> formatting is a little nicer in the PR.
>

This looks very good! I was thinking if it would be possible to reference
other targets (i.e. artifacts) from scripts but with the latest addition of
shared folder this does not seem to be a problem since it can be also
shared with another newt build invoked from script and we can copy/write
data there. I did not yet check how this work in practice but will give it
a try and perhaps then I'll have some extra ideas.


> Thanks,
> Chris
>
> ---
>
> A package specifies custom commands in its `pkg.yml` file.  There are
> two types of commands: 1) pre_cmds (run before the build), and 2)
> post_cmds (run after the build).
>
> ### EXAMPLE
>
> Example (apps/blinky/pkg.yml):
>
> pkg.pre_cmds:
> scripts/pre_build1.sh: 100
> scripts/pre_build2.sh: 200
>
> pkg.post_cmds:
> scripts/post_build.sh: 100
>

I assume these are relative to package root so perhaps we could assume
there is scripts/ subdir and execute from there by default? Just the same
as we have src/ and include/.

Best,
Andrzej

Re: Newt feature: run custom commands at build time

[Christopher Collins]

On Wed, Sep 25, 2019 at 06:29:19PM +0300, marko kiiskila wrote:
> Sounds good. If we need to add more, we can do that later.

Thanks.  I have updated the PR with the discussed changes:


Chris

Re: Newt feature: run custom commands at build time

[marko kiiskila]

> On 24 Sep 2019, at 20.14, Christopher Collins  wrote:
> 
> Hi Marko,
> 
> On Tue, Sep 24, 2019 at 03:19:24PM +0300, marko kiiskila wrote:
>> Thanks, this is a very useful feature.
>> 
>>> On 24 Sep 2019, at 3.50, Christopher Collins  wrote:
>>> 
>> ...
>>> 
>>> A package specifies custom commands in its `pkg.yml` file.  There are
>>> two types of commands: 1) pre_cmds (run before the build), and 2)
>>> post_cmds (run after the build). 
>>> 
>>> ### EXAMPLE
>>> 
>>> Example (apps/blinky/pkg.yml):
>>> 
>>>   pkg.pre_cmds:
>>>   scripts/pre_build1.sh: 100
>>>   scripts/pre_build2.sh: 200
>>> 
>>>   pkg.post_cmds:
>>>   scripts/post_build.sh: 100
>>> 
>>> For each command, the string on the left specifies the command to run.
>>> The number on the right indicates the command's relative ordering.
>> 
>> I wasn’t sure about need for numbering, but I can see a use if execution
>> of custom commands depends on a syscfg variable being set.
> 
> I also wasn't sure about using numbered stages.  It would be simpler if
> each package specified a sequence of commands rather than a mapping.
> 
> I was envisioning a case where package A generates some intermediate
> file, then package B reads this file and produces the final output.  In
> this scenario, B's scripts must run after A's scripts.  Assigning stages
> to each command allows the user to enforce this ordering.  Yes you are
> right - packages would need to use syscfg settings to define the stages
> for this to be useful.
> 
> Another abstract use case: several scripts emit strings to some temp
> files, then a final script would gather these files and generate a .c /
> .h pair.  Admittedly, I can't think of a concrete example where this
> would be needed.  
> 
> My feeling is we should continue using numeric stages here because a)
> they could conceivably be useful, and b) it's already implemented this
> way :).  I'm really uncertain about this though, so if you (or anyone
> else) has a strong opinion about this, please just let me know and I'll
> go with it.

I do not have strong opinion about this one. I’m fine with the scheme
you have described.

> 
>> I think one useful addition would be to have another class of commands
>> for linking phase. Often there’s a need to convert the binary to some
>> other format, or slap in another header (like with DA1469x bootloader).
>> Maybe have such an option for BSP packages? pkg.post_cmds would
>> get executed after generating object files, and maybe pkg.post_link_cmds
>> would get executed after linking?
> 
> That is a good idea.  How about this naming scheme:
> 
>* pre_build_cmds
>* pre_link_cmds
>* post_build_cmds
> 
> ?

That seems reasonable.

> 
> I also think we would need some an additional environment variables to
> make this useful:
> 
>* MYNEWT_PKG_BIN_DIR(dir where .o / .a files get written)
>* MYNEWT_PKG_BIN_FILE   (full path of .a file)
> 
> While we are at it, might as well add one more:
> 
>* MYNEWT_USER_WORK_DIR  (temp dir shared by all scripts)
> 
> It might be useful for scripts that feed input to other scripts.
> 

Sounds good. If we need to add more, we can do that later.

—
M

Re: Newt feature: run custom commands at build time

[Christopher Collins]

Hi Marko,

On Tue, Sep 24, 2019 at 03:19:24PM +0300, marko kiiskila wrote:
> Thanks, this is a very useful feature.
> 
> > On 24 Sep 2019, at 3.50, Christopher Collins  wrote:
> > 
> ...
> > 
> > A package specifies custom commands in its `pkg.yml` file.  There are
> > two types of commands: 1) pre_cmds (run before the build), and 2)
> > post_cmds (run after the build). 
> > 
> > ### EXAMPLE
> > 
> > Example (apps/blinky/pkg.yml):
> > 
> >pkg.pre_cmds:
> >scripts/pre_build1.sh: 100
> >scripts/pre_build2.sh: 200
> > 
> >pkg.post_cmds:
> >scripts/post_build.sh: 100
> > 
> > For each command, the string on the left specifies the command to run.
> > The number on the right indicates the command's relative ordering.
> 
> I wasn’t sure about need for numbering, but I can see a use if execution
> of custom commands depends on a syscfg variable being set.

I also wasn't sure about using numbered stages.  It would be simpler if
each package specified a sequence of commands rather than a mapping.

I was envisioning a case where package A generates some intermediate
file, then package B reads this file and produces the final output.  In
this scenario, B's scripts must run after A's scripts.  Assigning stages
to each command allows the user to enforce this ordering.  Yes you are
right - packages would need to use syscfg settings to define the stages
for this to be useful.

Another abstract use case: several scripts emit strings to some temp
files, then a final script would gather these files and generate a .c /
.h pair.  Admittedly, I can't think of a concrete example where this
would be needed.  

My feeling is we should continue using numeric stages here because a)
they could conceivably be useful, and b) it's already implemented this
way :).  I'm really uncertain about this though, so if you (or anyone
else) has a strong opinion about this, please just let me know and I'll
go with it.

> I think one useful addition would be to have another class of commands
> for linking phase. Often there’s a need to convert the binary to some
> other format, or slap in another header (like with DA1469x bootloader).
> Maybe have such an option for BSP packages? pkg.post_cmds would
> get executed after generating object files, and maybe pkg.post_link_cmds
> would get executed after linking?

That is a good idea.  How about this naming scheme:

* pre_build_cmds
* pre_link_cmds
* post_build_cmds

?

I also think we would need some an additional environment variables to
make this useful:

* MYNEWT_PKG_BIN_DIR(dir where .o / .a files get written)
* MYNEWT_PKG_BIN_FILE   (full path of .a file)

While we are at it, might as well add one more:

* MYNEWT_USER_WORK_DIR  (temp dir shared by all scripts)

It might be useful for scripts that feed input to other scripts.

Chris

Re: Newt feature: run custom commands at build time

[marko kiiskila]

Thanks, this is a very useful feature.

> On 24 Sep 2019, at 3.50, Christopher Collins  wrote:
> 
...
> 
> A package specifies custom commands in its `pkg.yml` file.  There are
> two types of commands: 1) pre_cmds (run before the build), and 2)
> post_cmds (run after the build). 
> 
> ### EXAMPLE
> 
> Example (apps/blinky/pkg.yml):
> 
>pkg.pre_cmds:
>scripts/pre_build1.sh: 100
>scripts/pre_build2.sh: 200
> 
>pkg.post_cmds:
>scripts/post_build.sh: 100
> 
> For each command, the string on the left specifies the command to run.
> The number on the right indicates the command's relative ordering.

I wasn’t sure about need for numbering, but I can see a use if execution
of custom commands depends on a syscfg variable being set.

I think one useful addition would be to have another class of commands
for linking phase. Often there’s a need to convert the binary to some
other format, or slap in another header (like with DA1469x bootloader).
Maybe have such an option for BSP packages? pkg.post_cmds would
get executed after generating object files, and maybe pkg.post_link_cmds
would get executed after linking?