448829 - new scheme for oink/Build.incl.mk.in depreciating the used of -I- cmd line option for cpp

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Gilles J. Seguin]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008071615 Fedora/3.0.1-1.fc9 Firefox/3.0.1
Build Identifier: oink-stack current

present scheme used -I- which generate following message:
"cc1plus: note: obsolete option -I- used, please use -iquote instead"


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Gilles J. Seguin]

Attachment: hg diff file for oink/Build.incl.mk.in

Comment 2

[(dormant account)]

I support the underlying idea of the patch, but why did you have to move ELSA_O and XML_OBJS, etc?

I'd rather keep the diff to a minimum by keeping as much as possible of the original. Also, don't comment out lines. Delete them instead.

Comment 3

[Gilles J. Seguin]

Attachment: hg diff for Build.inl.mk.in

the patch should be applied after #331996

Comment 4

[Gilles J. Seguin]

(In reply to comment #2)
> I support the underlying idea of the patch, but why did you have to move ELSA_O
> and XML_OBJS, etc?

"simply expanded" variables, that is using ":=", was used to defined ELSA_O and XML_OBJS. Because the new scheme is getting ride of the "-I-" compile option, new symbolic links for the *.cc files are created.  The placement of the line

ElsaObjs.links: $(ELSA_O:.o=.cc)
govern where ELSA_O should be, that is before previous line.
Placement of that line may be matter of opinion. Tell me yours.

ELSA_O force XML_OBJS to be before is definition, since XML_OBJS is a "simply expanded" variable, that means it behaves like a variable in C++ language.


> I'd rather keep the diff to a minimum by keeping as much as possible of the
> original.
> 

logical arrangement of the Build.incl.mk.in file was preferred to minimizing size of diff patch.

> Also, don't comment out lines. Delete them instead.
>

The commented lines were kept for documentation purpose. Which is a matter of opinion.

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> (In reply to comment #2)
> ELSA_O force XML_OBJS to be before is definition, since XML_OBJS is a "simply
> expanded" variable, that means it behaves like a variable in C++ language.

Why is := necessary or desirable? In general we don't favor it over gmake's =.

> > I'd rather keep the diff to a minimum by keeping as much as possible of the
> > original.
> 
> logical arrangement of the Build.incl.mk.in file was preferred to minimizing
> size of diff patch.

Passive voice alert. You mean *you* preferred rearranging to minimizing. But your reviewer said explicitly (with active voice) that he'd rather minimize. I'm here to plead for cooperation.

Unless, of course, you plan to be the new maintainer of the file, in which case rearrange at will -- but still split up spot fixes from major overhaul work that rearranges beyond what the spot-fix requires.

> > Also, don't comment out lines. Delete them instead.
> 
> The commented lines were kept for documentation purpose. Which is a matter of
> opinion.

Beyond cooperation being a good in getting patches ready to commit, and asserting contrary opinions not being very cooperative, the documentation benefits of obsolete lines is marginal to counter-productive. We don't keep old lines around in comments. Hg (cvs before it) will remember. Documentation in comments should use prose.

/be

Comment 6

[Gilles J. Seguin]

second pass

Comment 7

[Gilles J. Seguin]

Attachment: hg diff file for oink/Build.incl.mk.in

Comment 8

[Joshua Cranmer [:jcranmer]]

At this time, Mozilla is no longer pursuing the use of Elsa and Pork for static analysis and rewriting. Therefore, there are no longer any intentions to fix or support any outstanding bugs in these tools.