mach build testing/ results in a full tree build

RESOLVED FIXED in mozilla25

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: markh)

Tracking

Trunk
mozilla25

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mach])

Attachments

(1 attachment, 1 obsolete attachment)

No description provided.
/testing doesn't contain a Makefile. The logic for |mach build| will walk parent dirs until it finds a Makefile. In this case, it grabs the top-level Makefile.

Unless you have an idea for better behavior, I'm tempted to mark this WONTFIX.
Flags: needinfo?(ehsan)
(In reply to Gregory Szorc [:gps] from comment #1)
> /testing doesn't contain a Makefile. The logic for |mach build| will walk
> parent dirs until it finds a Makefile. In this case, it grabs the top-level
> Makefile.

I see.

> Unless you have an idea for better behavior, I'm tempted to mark this
> WONTFIX.

A better behavior would be to do what the user would expect, and build only the subdirectories of testing/.
Flags: needinfo?(ehsan)
And what happens if you |mach build services/healthreport/tests/xpcshell|? This directory has no Makefile nor child directories. Are you suggesting we traverse either up or down depending on whether child directories are present?
(In reply to comment #3)
> And what happens if you |mach build services/healthreport/tests/xpcshell|? This
> directory has no Makefile nor child directories. Are you suggesting we traverse
> either up or down depending on whether child directories are present?

Hmm, good question.  I don't know, what I would expect from a sane build utility would be able to take any given path name, and build the minimum set of things in order to take the effect of changes to that directory into account.  Does that sound reasonable?
IMO it's reasonable to just report an error in the case where walking upwards to find a Makefile takes you to the root - just something simple like "mach can't find a suitable Makefile in directory '%s' - perhaps you meant to build one of its children?".  IMO, no need to elaborate or do anything smarter...
(In reply to Mark Hammond (:markh) from comment #5)
> IMO it's reasonable to just report an error in the case where walking
> upwards to find a Makefile takes you to the root - just something simple
> like "mach can't find a suitable Makefile in directory '%s' - perhaps you
> meant to build one of its children?".  IMO, no need to elaborate or do
> anything smarter...

That works for me. Code is at https://hg.mozilla.org/mozilla-central/file/default/python/mozbuild/mozbuild/mach_commands.py#l279 if you want to have a go at it!
something like this?  I'm really not sure if the 'if os.path.isdir(target)' is necessary or if the simple check for falsey make_dir and make_target is all we need.
Attachment #774512 - Flags: feedback?(gps)
Comment on attachment 774512 [details] [diff] [review]
avoid accidentally rebuilding the tree

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

Looks good!

::: python/mozbuild/mozbuild/mach_commands.py
@@ +296,5 @@
>  
> +                    # See bug 886162 - we don't want to "accidentally" build
> +                    # the entire tree (if that's really the intent, it's
> +                    # unlikely they would have specified a directory.)
> +                    if os.path.isdir(target) and not make_dir and not make_target:

I /think/ the os.path.isdir() can be omitted.

@@ +300,5 @@
> +                    if os.path.isdir(target) and not make_dir and not make_target:
> +                        print("Can't find a suitable makefile in '%s' - "
> +                              "perhaps you meant to build one of its children?"
> +                              % target)
> +                        return 1

I would rephrase slightly to let people know exactly what happened. e.g.

"The specified directory doesn't contain a Makefile and the first parent directory with one is the root of the tree. I'm assuming you don't want to build the whole tree and have cancelled the build. Please specify a directory with a Makefile or run |mach build| if you want to build the entire tree."

Maybe that can be more concise?
Attachment #774512 - Flags: feedback?(gps) → feedback+
Assignee: nobody → mhammond
Attachment #774512 - Attachment is obsolete: true
Attachment #775060 - Flags: review?(gps)
Comment on attachment 775060 [details] [diff] [review]
drop isdir check and updated message

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

::: python/mozbuild/mozbuild/mach_commands.py
@@ +299,5 @@
> +                    # unlikely they would have specified a directory.)
> +                    if not make_dir and not make_target:
> +                        print("The specified directory doesn't contain a "
> +                              "Makefile and the first parent with one is the "
> +                              "root of the tree.  Please specify a directory "

Nit: Only need single space after period. FWIW, writing style guidelines formerly recommend double spaces but have since changed in the last 5-10 years.
Attachment #775060 - Flags: review?(gps) → review+
Component: mach → Build Config
Whiteboard: [mach]
https://hg.mozilla.org/integration/mozilla-inbound/rev/f22202174cea
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/f22202174cea
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.