341223 - Make $MOZCONFIG try $topsrcdir-local paths too

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Håkan Waara]

I keep a bunch of different mozconfigs in my tree, so each day I manually set the one that I'll use that day. 

For example, in my tree I have mozconfig-firefox-cocoa, mozconfig-firefox-carbon, etc.

70% of the times, I forget that $MOZCONFIG is an absolute path, so I just:

export $MOZCONFIG=mozconfig-firefoz-cocoa , and it won't work. :-)

I'd like mozconfig to try the $topsrcdir to find the mozconfig if it doesn't succeed in the normal ways.

Comment 1

[Håkan Waara]

Attachment: Proposed fix

Comment 2

[Håkan Waara]

Comment on attachment 225253 [details] [diff] [review]
Proposed fix

Clearing review request.

Mento wanted me to take some other (more difficult) approach that I don't remember.

Comment 3

[Matthew Gertner]

This bug is a major nuisance since not finding MOZCONFIG means that mk options like OBJDIR are not found, resulting in a source tree build. There are bugs with distclean that mean that fixing a source tree after it gets polluted by object and makefiles is not entirely straightforward. If this is as easy to fix as Hakan suggests, I can't see any obvious downside.

Hakan, do you still not remember why your fix was not acceptable. Can anyone else enlighten?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: A slightly better solution

Make MOZCONFIG and MY_MOZCONFIG absolute path if the path is relative.

Actually this fix is not a perfect because it is effective only if the current directory is topsrcdir. 

e.g)

In topsrcdir, 

MOZCONFIG=./.mozconfig.local make -f client.mk build

It's OK.

But in the parent directory of topsrcdir,

MOZCONFIG=./.mozconfig.local make -f mozilla-central/client.mk build

It does not work.

But I think it is not a problem for now because the client.mk does not supposed to be worked except on topsrcdir.

Comment 5

[(not currently active) Ted Mielczarek]

client.mk should be able to correctly pass $topsrcdir to mozconfig-find:
http://mxr.mozilla.org/mozilla-central/source/client.mk#87

so this should work either way.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 547591 [details] [diff] [review]
A slightly better solution

Review of attachment 547591 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/autoconf/mozconfig-find
@@ +54,5 @@
> +  fi
> +}
> +
> +[ -n "$MOZCONFIG" ] && MOZCONFIG=`absolute_path "$MOZCONFIG"`
> +[ -n "$MOZ_MYCONFIG" ] && MOZ_MYCONFIG=`absolute_path "$MOZ_MYCONFIG"`

I think you should just remove all references to MOZ_MYCONFIG while you're here. I don't see why anyone would use that.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Revised patch

Ted, I am sorry that the previous patch does not work well if user specify 
MOZCONFIG as absolute path. I supposed I had tested, but I usually make lots of mistakes. So I did mistake something at that time.

Anyway this new patch removes MOZ_MYCONFIG and use '%%' to check whether the path is absolute or not.

This time I confirmed the following one liners.

In case of relative path:

 MOZCONFIG=.mozconfig-test sh -c 'touch $MOZCONFIG && PWD=`pwd` && FOUND=`./build/autoconf/mozconfig-find $PWD` && [ $FOUND = $PWD/$MOZCONFIG ] && echo 'OK'; rm $MOZCONFIG' 

In case of absolute path:
 MOZCONFIG=/tmp/.mozconfig-test sh -c 'touch $MOZCONFIG && FOUND=`./build/autoconf/mozconfig-find /tmp` && [ $FOUND = $MOZCONFIG ] && echo 'OK'; rm $MOZCONFIG'

Comment 8

[Ed Morley [:emorley]]

This landed as:
http://hg.mozilla.org/mozilla-central/rev/4d794b0ef38d

However, this change causes the following error on win32 using relative configure/pymake:

.././build/autoconf/mozconfig2configure: line 118: Specified MOZCONFIG "../C:\Users\Ed\.mozconfig" does not exist!: No such file or directory

Backing this out locally fixes my existing mozconfig.

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

this broke how I build as well, which is to have my mozconfigs in my objdir and a MOZCONFIG=./mozconfig env var.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #9)
> this broke how I build as well, which is to have my mozconfigs in my objdir
> and a MOZCONFIG=./mozconfig env var.

Is the build for Android?

Comment 11

[Ed Morley [:emorley]]

If it helps, my mozconfig is specified using absolute path via system environment variable $MOZCONFIG=C:\Users\Ed\.mozconfig

Comment 12

[Brad Lassey [:blassey] (use needinfo?)]

talked to hiro on #developers and agreed to back out http://hg.mozilla.org/mozilla-central/rev/a43c3080f472

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #10)
> (In reply to comment #9)
> > this broke how I build as well, which is to have my mozconfigs in my objdir
> > and a MOZCONFIG=./mozconfig env var.
> 
> Is the build for Android?

It is, but I'm also broken for x86 builds

Comment 14

[(not currently active) Ted Mielczarek]

(In reply to comment #9)
> this broke how I build as well, which is to have my mozconfigs in my objdir
> and a MOZCONFIG=./mozconfig env var.

FWIW, I think we might break this use case in the future. If we're going to support relative $MOZCONFIG pathnames, they're going to be srcdir-relative.

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #14)
> (In reply to comment #9)
> > this broke how I build as well, which is to have my mozconfigs in my objdir
> > and a MOZCONFIG=./mozconfig env var.
> 
> FWIW, I think we might break this use case in the future. If we're going to
> support relative $MOZCONFIG pathnames, they're going to be srcdir-relative.

that doesn't make much sense. The mozconfig has the configurations for that build, so specific to the objdir.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Attachment: Support msys path too

This patch supports full path on msys environment. e.g.

C:\Users\Ed\.mozconfig
/c/Users/Ed/.mozconfig
\\YourHost\.mozconfig

Comment 17

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #9)
> > > this broke how I build as well, which is to have my mozconfigs in my objdir
> > > and a MOZCONFIG=./mozconfig env var.
> > 
> > FWIW, I think we might break this use case in the future. If we're going to
> > support relative $MOZCONFIG pathnames, they're going to be srcdir-relative.
> 
> that doesn't make much sense. The mozconfig has the configurations for that
> build, so specific to the objdir.

But the current directory at that time is topsrcdir? If so , it is natual to use MOZCONFIG=objdir/mozconfig.

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #17)
> But the current directory at that time is topsrcdir? If so , it is natual to
> use MOZCONFIG=objdir/mozconfig.
I don't follow. My point is that the mozconfig is associated with the build, so it should be relative to the objdir, not the topsrcdir.

Comment 19

[Justin Wood (:Callek)]

Blassey, actually the mozconfig is associated with the _run_ we are on. Since note client.mk uses it.

*and* we have always supported |mk_add_option MOZ_OBJDIR=FOO| and one common foo there is

|mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@| which is mentioned on the docs at MDC

If someone can specify relative dir's, the two spots this would get invoked from would differ in where to find it. (We can't relatively search for a file from something that the file itself can identify a location for)

So I'd say that relatively going from objdir is a no-go. As useful as it feels in theory.

Comment 20

[Rain [:rain1] (inactive)]

cf bug 675691 for where I think we should be going.

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 550282 [details] [diff] [review]
Support msys path too

Review of attachment 550282 [details] [diff] [review]:
-----------------------------------------------------------------

I hate shell scripting. :-(

Comment 22

[Justin Wood (:Callek)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2b05e85795d1

Comment 23

[Mounir Lamouri (:mounir)]

Backed-out from inbound due to a perma-orange, see:
http://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=0adaae140642

The patch in this bug might not bee related, the entire push has been backed out.

Comment 24

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

That patch still breaks me. Its not clear what this change improves, but if you're going to do it, how about you not break my build?

Comment 25

[Hiroyuki Ikezoe (:hiro)]

(In reply to Brad Lassey [:blassey][[e-mail]] from comment #24)
> Created attachment 551773 [details] [diff] [review] [diff] [details] [review]
> patch
> 
> That patch still breaks me. Its not clear what this change improves, but if
> you're going to do it, how about you not break my build?

If we can use a magic keyword which represents $top_buildir (i.e. MOZ_OBJDIR) in shell, it must be a solution for you.

Comment 26

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)
> If we can use a magic keyword which represents $top_buildir (i.e.
> MOZ_OBJDIR) in shell, it must be a solution for you.
The patch I attached works

Comment 27

[Hiroyuki Ikezoe (:hiro)]

Just out of curiosity, what is your MOZCONFIG when fresh building? You make a MOZ_OBJDIR and create a .mozconfig in there first?

Comment 28

[Brad Lassey [:blassey] (use needinfo?)]

yup, I do:
$> mkdir objdir-droid
$> emacs objdir-droid/mozconfig

Comment 29

[Hiroyuki Ikezoe (:hiro)]

(In reply to Brad Lassey [:blassey][[e-mail]] from comment #28)
> yup, I do:
> $> mkdir objdir-droid
> $> emacs objdir-droid/mozconfig

Thanks.

IMHO, these steps before building and set env value (in this case it's MOZCONFIG) relative to target (building) directory are weird at least for me (Especially second one). But I leave the decision up to other developers.

Comment 30

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #29)
> IMHO, these steps before building and set env value (in this case it's
> MOZCONFIG) relative to target (building) directory are weird at least for me
> (Especially second one). But I leave the decision up to other developers.

That's fine, you don't have to build that way. But why break it when you don't have to?

Also, as I said in comment 24, I don't see what value this bug/patch adds. What problem are you trying to fix?

Comment 31

[Justin Wood (:Callek)]

(In reply to Brad Lassey [:blassey][[e-mail]] from comment #30)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #29)
> > IMHO, these steps before building and set env value (in this case it's
> > MOZCONFIG) relative to target (building) directory are weird at least for me
> > (Especially second one). But I leave the decision up to other developers.
> 
> That's fine, you don't have to build that way. But why break it when you
> don't have to?
> 
> Also, as I said in comment 24, I don't see what value this bug/patch adds.
> What problem are you trying to fix?

I feel relative to objdir is counter to what we were/are doing here now, and counter to what we have always recommended/supported. I'll let ted make the call though, the original patch here *had* review, which is why I landed it. But I'm surely not module-owner.

Comment 32

[Rain [:rain1] (inactive)]

I just uploaded a patch to bug 675691 that goes in the other direction, removing , btw. I think this bug should be wontfix'd (with my reasoning in bug 675691 comment 0).

Comment 33

[Rain [:rain1] (inactive)]

> removing , 

er, I meant "removing things to guess".

Comment 34

[(not currently active) Ted Mielczarek]

Comment on attachment 551773 [details] [diff] [review]
patch

Review of attachment 551773 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think we're going to take this. I actually think we're going to take the patch in bug 675691 instead.

Comment 35

[Rain [:rain1] (inactive)]

I believe we're not going to do this.