[commit: ghc] master: Make the rule for .hi files depend on the .hs/.lhs files (5cb0880)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[commit: ghc] master: Make the rule for .hi files depend on the .hs/.lhs files (5cb0880)

Simon Marlow-7
On 10/01/13 19:24, Ian Lynagh wrote:

> Repository : ssh://darcs.haskell.org//srv/darcs/ghc
>
> On branch  : master
>
> http://hackage.haskell.org/trac/ghc/changeset/5cb088088be8573a01c991421ad882ce899067db
>
>> ---------------------------------------------------------------
>
> commit 5cb088088be8573a01c991421ad882ce899067db
> Author: Ian Lynagh <ian at well-typed.com>
> Date:   Sat Jan 5 18:50:29 2013 +0000
>
>      Make the rule for .hi files depend on the .hs/.lhs files
>
>      make thought that it could make a .hi file for the C files in
>      libraries, which was causing problems when using dynamic-too.

Are you sure?  This rule is *very* delicate (see the comments).  Won't
this change cause make to think that it can build the .hi file by not
doing anything?

Cheers,
        Simon


>> ---------------------------------------------------------------
>
>   ghc.mk                          |    4 ----
>   rules/hi-rule.mk                |    8 +++++---
>   rules/hs-suffix-rules-srcdir.mk |    2 ++
>   rules/hs-suffix-rules.mk        |    3 +++
>   4 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/ghc.mk b/ghc.mk
> index f4a7a61..d71d8c9 100644
> --- a/ghc.mk
> +++ b/ghc.mk
> @@ -229,10 +229,6 @@ ifneq "$(CLEANING)" "YES"
>   include rules/hs-suffix-rules-srcdir.mk
>   include rules/hs-suffix-rules.mk
>   include rules/hi-rule.mk
> -
> -$(foreach way,$(ALL_WAYS),\
> -  $(eval $(call hi-rule,$(way))))
> -
>   include rules/c-suffix-rules.mk
>   include rules/cmm-suffix-rules.mk
>
> diff --git a/rules/hi-rule.mk b/rules/hi-rule.mk
> index 35baffd..c1e7502 100644
> --- a/rules/hi-rule.mk
> +++ b/rules/hi-rule.mk
> @@ -62,11 +62,13 @@
>   # documentation).  An empty command is enough to get GNU make to think
>   # it has updated %.hi, but without actually spawning a shell to do so.
>
> -define hi-rule # $1 = way
> +define hi-rule # $1 = source directory, $2 = object directory, $3 = way
>
> -%.$$($1_hisuf) : %.$$($1_osuf) ;
> +$2/%.$$($3_hisuf) : $2/%.$$($3_osuf) $1/%.hs ;
> +$2/%.$$($3_hisuf) : $2/%.$$($3_osuf) $1/%.lhs ;
>
> -%.$$($1_way_)hi-boot : %.$$($1_way_)o-boot ;
> +$2/%.$$($3_way_)hi-boot : $2/%.$$($3_way_)o-boot $1/%.hs ;
> +$2/%.$$($3_way_)hi-boot : $2/%.$$($3_way_)o-boot $1/%.lhs ;
>
>   endef
>
> diff --git a/rules/hs-suffix-rules-srcdir.mk b/rules/hs-suffix-rules-srcdir.mk
> index 94a41d5..776d1ce 100644
> --- a/rules/hs-suffix-rules-srcdir.mk
> +++ b/rules/hs-suffix-rules-srcdir.mk
> @@ -52,6 +52,8 @@ $1/$2/build/%.$$($3_hcsuf) : $1/$4/%.hs $$(LAX_DEPS_FOLLOW) $$($1_$2_HC_DEP) $$(
>   $1/$2/build/%.$$($3_hcsuf) : $1/$4/%.lhs $$(LAX_DEPS_FOLLOW) $$($1_$2_HC_DEP) $$($1_$2_PKGDATA_DEP)
>   $$(call cmd,$1_$2_HC) $$($1_$2_$3_ALL_HC_OPTS) -C $$< -o $$@
>
> +$(call hi-rule,$1/$4,$1/$2/build,$3)
> +
>   endif
>
>   # XXX: for some reason these get used in preference to the direct
> diff --git a/rules/hs-suffix-rules.mk b/rules/hs-suffix-rules.mk
> index 9d54753..9b11e6e 100644
> --- a/rules/hs-suffix-rules.mk
> +++ b/rules/hs-suffix-rules.mk
> @@ -28,6 +28,9 @@ $1/$2/build/%.$$($3_hcsuf) : $1/$2/build/autogen/%.hs $$(LAX_DEPS_FOLLOW) $$($1_
>   $1/$2/build/%.$$($3_osuf) : $1/$2/build/autogen/%.hs $$(LAX_DEPS_FOLLOW) $$($1_$2_HC_DEP)
>   $$(call cmd,$1_$2_HC) $$($1_$2_$3_ALL_HC_OPTS) -c $$< -o $$@
>
> +$(call hi-rule,$1/$2/build,$1/$2/build,$3)
> +$(call hi-rule,$1/$2/build/autogen,$1/$2/build,$3)
> +
>   endif
>   endif
>
>
>
>
> _______________________________________________
> ghc-commits mailing list
> ghc-commits at haskell.org
> http://www.haskell.org/mailman/listinfo/ghc-commits
>



Reply | Threaded
Open this post in threaded view
|

[commit: ghc] master: Make the rule for .hi files depend on the .hs/.lhs files (5cb0880)

Ian Lynagh-2
On Thu, Jan 10, 2013 at 08:44:21PM +0000, Simon Marlow wrote:

> On 10/01/13 19:24, Ian Lynagh wrote:
> >
> >commit 5cb088088be8573a01c991421ad882ce899067db
> >Author: Ian Lynagh <ian at well-typed.com>
> >Date:   Sat Jan 5 18:50:29 2013 +0000
> >
> >     Make the rule for .hi files depend on the .hs/.lhs files
> >
> >     make thought that it could make a .hi file for the C files in
> >     libraries, which was causing problems when using dynamic-too.
>
> Are you sure?  This rule is *very* delicate (see the comments).
> Won't this change cause make to think that it can build the .hi file
> by not doing anything?

I hesitate to say I'm sure, but I don't see the problem.

The old rule was roughly:

    %.hi: %.o ;

and I could break the change into 2 parts, first changing it to
something like:

    a/%.hi: a/%.o ;

and then to something like:

    a/%.hi: a/%.o b/%.hs ;

Which step do you think is wrong, and why?

> >+define hi-rule # $1 = source directory, $2 = object directory, $3 = way
> >
> >-%.$$($1_hisuf) : %.$$($1_osuf) ;
> >+$2/%.$$($3_hisuf) : $2/%.$$($3_osuf) $1/%.hs ;


Thanks
Ian



Reply | Threaded
Open this post in threaded view
|

[commit: ghc] master: Make the rule for .hi files depend on the .hs/.lhs files (5cb0880)

Simon Marlow-7
In reply to this post by Simon Marlow-7
On 10/01/13 20:44, Simon Marlow wrote:

> On 10/01/13 19:24, Ian Lynagh wrote:
>> Repository : ssh://darcs.haskell.org//srv/darcs/ghc
>>
>> On branch  : master
>>
>> http://hackage.haskell.org/trac/ghc/changeset/5cb088088be8573a01c991421ad882ce899067db
>>
>>
>>> ---------------------------------------------------------------
>>
>> commit 5cb088088be8573a01c991421ad882ce899067db
>> Author: Ian Lynagh <ian at well-typed.com>
>> Date:   Sat Jan 5 18:50:29 2013 +0000
>>
>>      Make the rule for .hi files depend on the .hs/.lhs files
>>
>>      make thought that it could make a .hi file for the C files in
>>      libraries, which was causing problems when using dynamic-too.
>
> Are you sure?  This rule is *very* delicate (see the comments).  Won't
> this change cause make to think that it can build the .hi file by not
> doing anything?

Never mind - I see that we still have the .o dependency, the .hs/.lhs
dependency was just added.

Still, I think an addition to the comments in the file to describe the
reasoning would be a good idea (right now the comments don't match the
code).

Incidentally, the sanity check (hi-rule-helper) increases the time to do
a no-op make from 3.3s to 4.9s here.  I'm not sure it's worth having on
by default, even on non-Windows, because of the impact on interactive
development, and if the .hi file doesn't exist, something will go badly
wrong very soon anyway.

Cheers,
        Simon



> Cheers,
>      Simon
>
>
>>> ---------------------------------------------------------------
>>
>>   ghc.mk                          |    4 ----
>>   rules/hi-rule.mk                |    8 +++++---
>>   rules/hs-suffix-rules-srcdir.mk |    2 ++
>>   rules/hs-suffix-rules.mk        |    3 +++
>>   4 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/ghc.mk b/ghc.mk
>> index f4a7a61..d71d8c9 100644
>> --- a/ghc.mk
>> +++ b/ghc.mk
>> @@ -229,10 +229,6 @@ ifneq "$(CLEANING)" "YES"
>>   include rules/hs-suffix-rules-srcdir.mk
>>   include rules/hs-suffix-rules.mk
>>   include rules/hi-rule.mk
>> -
>> -$(foreach way,$(ALL_WAYS),\
>> -  $(eval $(call hi-rule,$(way))))
>> -
>>   include rules/c-suffix-rules.mk
>>   include rules/cmm-suffix-rules.mk
>>
>> diff --git a/rules/hi-rule.mk b/rules/hi-rule.mk
>> index 35baffd..c1e7502 100644
>> --- a/rules/hi-rule.mk
>> +++ b/rules/hi-rule.mk
>> @@ -62,11 +62,13 @@
>>   # documentation).  An empty command is enough to get GNU make to think
>>   # it has updated %.hi, but without actually spawning a shell to do so.
>>
>> -define hi-rule # $1 = way
>> +define hi-rule # $1 = source directory, $2 = object directory, $3 = way
>>
>> -%.$$($1_hisuf) : %.$$($1_osuf) ;
>> +$2/%.$$($3_hisuf) : $2/%.$$($3_osuf) $1/%.hs ;
>> +$2/%.$$($3_hisuf) : $2/%.$$($3_osuf) $1/%.lhs ;
>>
>> -%.$$($1_way_)hi-boot : %.$$($1_way_)o-boot ;
>> +$2/%.$$($3_way_)hi-boot : $2/%.$$($3_way_)o-boot $1/%.hs ;
>> +$2/%.$$($3_way_)hi-boot : $2/%.$$($3_way_)o-boot $1/%.lhs ;
>>
>>   endef
>>
>> diff --git a/rules/hs-suffix-rules-srcdir.mk
>> b/rules/hs-suffix-rules-srcdir.mk
>> index 94a41d5..776d1ce 100644
>> --- a/rules/hs-suffix-rules-srcdir.mk
>> +++ b/rules/hs-suffix-rules-srcdir.mk
>> @@ -52,6 +52,8 @@ $1/$2/build/%.$$($3_hcsuf) : $1/$4/%.hs
>> $$(LAX_DEPS_FOLLOW) $$($1_$2_HC_DEP) $$(
>>   $1/$2/build/%.$$($3_hcsuf) : $1/$4/%.lhs $$(LAX_DEPS_FOLLOW)
>> $$($1_$2_HC_DEP) $$($1_$2_PKGDATA_DEP)
>>       $$(call cmd,$1_$2_HC) $$($1_$2_$3_ALL_HC_OPTS) -C $$< -o $$@
>>
>> +$(call hi-rule,$1/$4,$1/$2/build,$3)
>> +
>>   endif
>>
>>   # XXX: for some reason these get used in preference to the direct
>> diff --git a/rules/hs-suffix-rules.mk b/rules/hs-suffix-rules.mk
>> index 9d54753..9b11e6e 100644
>> --- a/rules/hs-suffix-rules.mk
>> +++ b/rules/hs-suffix-rules.mk
>> @@ -28,6 +28,9 @@ $1/$2/build/%.$$($3_hcsuf) :
>> $1/$2/build/autogen/%.hs $$(LAX_DEPS_FOLLOW) $$($1_
>>   $1/$2/build/%.$$($3_osuf) : $1/$2/build/autogen/%.hs
>> $$(LAX_DEPS_FOLLOW) $$($1_$2_HC_DEP)
>>       $$(call cmd,$1_$2_HC) $$($1_$2_$3_ALL_HC_OPTS) -c $$< -o $$@
>>
>> +$(call hi-rule,$1/$2/build,$1/$2/build,$3)
>> +$(call hi-rule,$1/$2/build/autogen,$1/$2/build,$3)
>> +
>>   endif
>>   endif
>>
>>
>>
>>
>> _______________________________________________
>> ghc-commits mailing list
>> ghc-commits at haskell.org
>> http://www.haskell.org/mailman/listinfo/ghc-commits
>>
>