patchmake - Patches: patch #6204, Merge redundant variable...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #6204: Merge redundant variable declarations in the Makefile example

Submitter:  Diego Biurrun <dondiego>
Submitted:  Mon 17 Sep 2007 11:37:43 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Fixed Release:  None

Jump to the original submission

Sun 18 Jul 2010 11:47:11 PM UTC, comment #6: 

Applied this, thanks.

Paul D. Smith <psmith>
Group administrator
Fri 28 Sep 2007 08:12:27 AM UTC, comment #5: 

Hmm, are there no further comments or suggestions for improvements?

Diego Biurrun <dondiego>
Mon 17 Sep 2007 02:46:46 PM UTC, comment #4: 

Here is a new attempt, hopefully you will like this one better.  It splits the sources by filetype and names them accordingly, not randomly 1, 2, 3.  This can then be exploited to create the OBJS variable automatically.

(file #13972)

Diego Biurrun <dondiego>
Mon 17 Sep 2007 01:52:44 PM UTC, comment #3: 

I understand what you mean and I said "I agree that [...] are not good names for variables".  However as I said I think this whole section should be replaced with something that reflects more of the capabilities of, and good style using, the current GNU make (although I don't want it to be so confusing no one can understand it without pages of explanatory text, either), rather than the existing 1980's era makefile example.

Paul D. Smith <psmith>
Group administrator
Mon 17 Sep 2007 01:30:02 PM UTC, comment #2: 

I will try to clarify a bit more what this patch is about: Of course it can be reasonable to have multiple variables if they are being grouped together in some way.  And of course it can be reasonable to combine them as shown in this way.  However, this is not what is done in the example. Let us look at the code in question:

SRC1 =  tar.c create.c extract.c buffer.c \
        getoldopt.c update.c gnu.c mangle.c
SRC2 =  version.c list.c names.c diffarch.c \
        port.c wildmat.c getopt.c
SRC3 =  getopt1.c regex.c getdate.y
SRCS =  $(SRC1) $(SRC2) $(SRC3)

The grouping is arbitrary AFAICT and the variable names are bad. Furthermore, the numbered variables are not used anywhere outside of the SRCS line.  This makes the indirection useless and confusing.  My first reaction to seeing this was to look for the place where the numbered variables were used again.

If you wish to have variable combination in the example Makefile then I will gladly come up with a better alternative.  However, I insist that the current form of pointless indirection is bad style and potentially confusing to novice make programmers.  An example that comes with the official documentation should be straightforward and showcase best practice programming.  Thus I ask you to please apply my patch, it is an improvement over the current situation.

Diego Biurrun <dondiego>
Mon 17 Sep 2007 12:21:14 PM UTC, comment #1: 

I don't necessarily agree with you.  It's quite reasonable to have multiple variables if they are being grouped together in some way, such as all 3rd party sources in one, etc.  It's also reasonable to combine them as shown here.  I do agree that "SRC1", "SRC2", and "SRC3" are not good names for variables in that case.

However, I will admit that this makefile example, which was added verbatim from the GNU tar package long before autoconf and automake existed, is dated and should be reworked to use more of the modern facilities of make.

So, I'll leave this open although I don't agree that the change in your patch is the correct one.

Paul D. Smith <psmith>
Group administrator
Mon 17 Sep 2007 11:37:43 AM UTC, original submission:  

The Makefile example in the appendix declares three variables SRC1, SRC2 and SRC3, only to merge them into one immediately afterwards:

SRCS =  $(SRC1) $(SRC2) $(SRC3)

This is redundant and arguably also bad style.  Here is a patch that simplifies this by directly declaring SRCS instead.

The same applies to OBJ1, OBJ2, OBJ3 and OBJS.

Diego Biurrun <dondiego>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #13972:  cleanup_variable_handling_in_example.diff added by dondiego (1KiB - text/x-diff - Merge variables and construct OBJS variable from SRCS automatically.)
file #13969:  redundant_variables_in_example.diff added by dondiego (1KiB - text/x-diff - Merges redundant variables into one.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dondiego (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-07-18 psmith StatusNone Done
        Open/ClosedOpen Closed
    2007-09-17 dondiego Attached File- Added cleanup_variable_handling_in_example.diff, #13972
    2007-09-17 dondiego Attached File- Added redundant_variables_in_example.diff, #13969

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code