Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify successful build message #62045

Closed
brettcannon opened this issue Apr 25, 2013 · 20 comments
Closed

Clarify successful build message #62045

brettcannon opened this issue Apr 25, 2013 · 20 comments
Assignees
Labels
build The build process and cross-build easy type-feature A feature request or enhancement

Comments

@brettcannon
Copy link
Member

BPO 17845
Nosy @brettcannon, @ezio-melotti, @merwok, @bitdancer, @demianbrecht
Files
  • issue17845.patch: Patch modifies success message on build completion
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/brettcannon'
    closed_at = <Date 2013-07-12.15:31:05.458>
    created_at = <Date 2013-04-25.17:44:34.768>
    labels = ['easy', 'type-feature', 'build']
    title = 'Clarify successful build message'
    updated_at = <Date 2013-07-12.15:31:05.457>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2013-07-12.15:31:05.457>
    actor = 'brett.cannon'
    assignee = 'brett.cannon'
    closed = True
    closed_date = <Date 2013-07-12.15:31:05.458>
    closer = 'brett.cannon'
    components = ['Build']
    creation = <Date 2013-04-25.17:44:34.768>
    creator = 'brett.cannon'
    dependencies = []
    files = ['30758']
    hgrepos = []
    issue_num = 17845
    keywords = ['patch', 'easy']
    message_count = 20.0
    messages = ['187794', '187795', '187796', '187797', '187798', '187800', '187801', '187802', '187803', '187804', '187805', '187806', '187808', '187809', '187816', '187826', '187832', '192241', '192312', '192948']
    nosy_count = 8.0
    nosy_names = ['brett.cannon', 'ezio.melotti', 'eric.araujo', 'r.david.murray', 'python-dev', 'demian.brecht', 'harrison.morgan', 'Yogesh.Chaudhari']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17845'
    versions = ['Python 3.4']

    @brettcannon
    Copy link
    Member Author

    From:

    Python build finished, but the necessary bits to build these modules were not found:
    ossaudiodev spwd
    To find the necessary bits, look in setup.py in detect_modules() for the module's name.

    To:

    Python build successfully finished, but the necessary bits to build these optional modules were not found:

    Notice the addition of "successfully" and "optional". Hopefully this will cause fewer new contributors to get thrown by this message.

    @brettcannon brettcannon added the build The build process and cross-build label Apr 25, 2013
    @brettcannon
    Copy link
    Member Author

    And just FYI, the pre-existing sentence already extends past 80 characters (84), so the new length of 104 shouldn't be a concern. Although we could re-format it into two lines::

    Python build finished successfully!
    The necessary bits to build these optional modules were not found:

    @brettcannon
    Copy link
    Member Author

    And I would probably go with "finished successfully" instead of "successfully finished".

    @merwok
    Copy link
    Member

    merwok commented Apr 25, 2013

    Using two lines sounds good, especially if the last one printed is the positive one (“build successful”).

    Do you think there will be oppotions to backporting this?

    @merwok
    Copy link
    Member

    merwok commented Apr 25, 2013

    opposition* :)

    @brettcannon
    Copy link
    Member Author

    Can't backport; someone might be relying on the output to verify their automated build successfully built or something.

    @harrisonmorgan
    Copy link
    Mannequin

    harrisonmorgan mannequin commented Apr 25, 2013

    As someone trying to get started contributing, I think this change makes it a good deal clearer (although at this point I already know that those modules are optional). The two line version looks better to me.

    However, "necessary bits" still seems unclear. Would it be going too far to suggest that it gets changed to something like this?

    Python build finished successfully!
    The necessary third-party packages to build these optional modules were not found:

    @merwok
    Copy link
    Member

    merwok commented Apr 25, 2013

    Brett: You’re right, too bad.

    Harrison: “third-party packages” may be ambiguous (Python distributions vs. system dependencies), and “required” may conflict with “optional”.

    I propose:

    Some optional modules were not built because of missing system files:
    ...
    Python interpreter built successfully!

    @brettcannon
    Copy link
    Member Author

    I guess the question is whether all the code is third-party or simply optional on some OS? Don't know the answer off the top of my head.

    @brettcannon
    Copy link
    Member Author

    I personally don't like the message re-ordering. It feels like "oops, you didn't build everything. Hey everything built fine!" It reads like there was a bug and we accidentally interpreted it as a success.

    And it isn't necessarily system files. I mean sqlite3 is not a system package, it's a DB project that we happen to support.

    @merwok
    Copy link
    Member

    merwok commented Apr 25, 2013

    I guess the question is whether all the code is third-party or simply
    optional on some OS? Don't know the answer off the top of my head.
    In my Debian world it’s typical to use only the official repos, there are no third parties (except from the viewpoint of Python). I used “system” instead of “third-party” in my proposition. For me on Debian the optional deps really are “not installed at Python build time” (“not installed by default” or “optional” doesn’t really make sense, as there are multiple defaults, and 98% of the packages are not required to have a working system).

    I personally don't like the message re-ordering. It feels like "oops, you
    didn't build everything. Hey everything built fine!"
    Well I distinguished “interpreter” and “optional modules” on purpose, not “everything”, but I agree it can be a fine distinction when you’re just getting started. Let’s not reorder the messages.

    And it isn't necessarily system files. I mean sqlite3 is not a system package
    Really? If it doesn’t build because sqlite3 header files are not found, then that’s a missing system file to me.

    (I used “file” instead of “package” because not everybody will install deps using the system package manager (if any), but that may be complicating things for no good reason.)

    @brettcannon
    Copy link
    Member Author

    I use homebrew on OS X and have it in a non-standard location which is not a systems directory (e.g. Developer/). When I hear "system", I think /usr, etc. which is not where people necessarily install third-party stuff.

    @harrisonmorgan
    Copy link
    Mannequin

    harrisonmorgan mannequin commented Apr 25, 2013

    Would "external libraries" work better? It's clearly not referring to Python packages. And could be installed by a system package manager, or by yourself in a non-standard location.

    Python build finished successfully!
    The necessary external libraries to build these optional modules were not found:

    @brettcannon
    Copy link
    Member Author

    "External libraries" works for me.

    @bitdancer
    Copy link
    Member

    I'm afraid that "External libraries" is still misleading. On package-manager-managed linux systems, it is often only the header files that are missing, the libraries are there. It may well be that "necessary bits" is the most informative choice :).

    With two lines I suppose you could be wordy and say "external libraries and/or header files", but that might take us beyond column 80 again.

    @harrisonmorgan
    Copy link
    Mannequin

    harrisonmorgan mannequin commented Apr 26, 2013

    Perhaps "necessary bits" really is the best way to put it. Here's one more suggestion, though:

    Python build finished successfully!
    External libraries and/or header files needed to build these optional modules were not found:

    Dropping the definite article and rearranging it to use "needed" instead of "necessary" keeps it at 93 characters, only 9 more than the current message. I'm not sure, but I think the "and/" could be dropped, too.

    That, at least, gives you something to put into Google.

    @ezio-melotti
    Copy link
    Member

    See also bpo-13472 for a related discussion.

    @ezio-melotti ezio-melotti added easy type-feature A feature request or enhancement labels Apr 26, 2013
    @brettcannon brettcannon self-assigned this Jul 1, 2013
    @YogeshChaudhari
    Copy link
    Mannequin

    YogeshChaudhari mannequin commented Jul 3, 2013

    Patch to modify setup.py comments on successful build

    @brettcannon
    Copy link
    Member Author

    First, thanks to Yogesh for writing a patch!

    Second, I still think the second line should be "The necessary bits to build these optional modules were not found:". I purposefully like the vagueness of it so we don't start going on about external vs. system, whether header files needs to be mentioned, etc. That is a perk for people who aren't C programmers and don't want to know what a header file is.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 12, 2013

    New changeset e13ff9fdfaf9 by Brett Cannon in branch 'default':
    Issue bpo-17845: Clarify the message setup.py prints upon successfully
    http://hg.python.org/cpython/rev/e13ff9fdfaf9

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build easy type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants