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

"make -j9 install" fails because bininstall target requires the libainstall target #69882

Closed
vstinner opened this issue Nov 22, 2015 · 12 comments
Labels
build The build process and cross-build

Comments

@vstinner
Copy link
Member

BPO 25696
Nosy @doko42, @vstinner, @vadmium
Files
  • bininstall.patch
  • bininstall-2.patch
  • bininstall-3.patch
  • bininstall-4.patch: Separate $(LIBPC) rule
  • 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 = None
    closed_at = <Date 2015-12-13.20:27:29.933>
    created_at = <Date 2015-11-22.14:31:42.825>
    labels = ['build']
    title = '"make -j9 install" fails because bininstall target requires the libainstall target'
    updated_at = <Date 2015-12-13.20:27:29.932>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2015-12-13.20:27:29.932>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-12-13.20:27:29.933>
    closer = 'vstinner'
    components = ['Build']
    creation = <Date 2015-11-22.14:31:42.825>
    creator = 'vstinner'
    dependencies = []
    files = ['41124', '41162', '41171', '41178']
    hgrepos = []
    issue_num = 25696
    keywords = ['patch']
    message_count = 12.0
    messages = ['255102', '255103', '255108', '255359', '255393', '255462', '255519', '255575', '255599', '256334', '256335', '256336']
    nosy_count = 5.0
    nosy_names = ['doko', 'vstinner', 'Arfrever', 'python-dev', 'martin.panter']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue25696'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

    @vstinner
    Copy link
    Member Author

    I'm unable to install Python 3.6, "make install" doesn't copy .py files because the bininstall target failed. It failed because /opt/py36/lib/pkgconfig doesn't exist.

    Try:
    ---
    export MAKEFLAGS=-j9
    rm -rf /opt/py36
    ./configure --prefix=/opt/py36
    make
    make install # you may need 'sudo make install'
    ---

    Attached patch fixes the Makefile, but I don't know if my change is correct.

    @vstinner vstinner added the build The build process and cross-build label Nov 22, 2015
    @vstinner
    Copy link
    Member Author

    Output:
    ---

    $ make install --debug=v
    (...)
      Must remake target 'bininstall'.
    (...)
            (cd /opt/py36/lib/pkgconfig; ln -s python-3.6.pc python-3.6dm.pc); (...)
    (...)
    /bin/sh: ligne 4 : cd: /opt/py36/lib/pkgconfig: Aucun fichier ou dossier de ce type

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Nov 22, 2015

    This problem in bininstall target seems to exist since Python 2.7 and 3.1.

    bininstall target depends on altbininstall target, which contains:

        @for i in $(BINDIR) $(LIBDIR); \
        do \
                if test ! -d $(DESTDIR)$$i; then \
                        echo "Creating directory $$i"; \
                        $(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
                else    true; \
                fi; \
        done
    

    For consistency with other things, I would suggest to add the following at the beginning of bininstall target (instead of making bininstall target depend on libainstall target):

        @for i in $(LIBPC); \
        do \
                if test ! -d $(DESTDIR)$$i; then \
                        echo "Creating directory $$i"; \
                        $(INSTALL) -d -m $(DIRMODE) $(DESTDIR)$$i; \
                else    true; \
                fi; \
        done
    

    @vstinner
    Copy link
    Member Author

    To be clear: the bug doesn't occur when Python is not installed "in parallel" (without MAKEFLAGS=-j9).

    "For consistency with other things, I would suggest to add the following at the beginning of bininstall target (instead of making bininstall target depend on libainstall target):"

    Ok. Does my new patch look better?

    Should I apply my fix to Python 2.7, 3.4, 3.5 and default (3.6)?

    @vstinner vstinner changed the title "make install" fails because bininstall target requires the libainstall target "MAKEFLAGS=-j9 make install" fails because bininstall target requires the libainstall target Nov 25, 2015
    @vstinner vstinner changed the title "MAKEFLAGS=-j9 make install" fails because bininstall target requires the libainstall target "make -j9 install" fails because bininstall target requires the libainstall target Nov 25, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 25, 2015

    $(LIBPC) is a single directory name, so I suggest dropping the “for” loop.

    Patch 2 should avoid the practical race condition. But there is technically still a race with “libainstall” and “bininstall” both testing and creating the same directory. Maybe we should factor it out, something like:

    $(DESTDIR)$(LIBPC):
    @echo "Creating directory $@"
    @$(INSTALL) -d -m $(DIRMODE) $@
    . . .
    bininstall: altbininstall $(DESTDIR)$(LIBPC)
    . . .
    libainstall: all python-config $(DESTDIR)$(LIBPC)
    @for i in $(LIBDIR) $(LIBPL); \
    . . .

    Looking at the history, the test-then-install code comes from 20 years ago: <https://hg.python.org/cpython/rev/d41d89eb43e5#l1.150\>. I would say calling install unconditionally (without a test or echo message) might be simpler, but that’s probably getting out of scope here.

    @vstinner
    Copy link
    Member Author

    $(LIBPC) is a single directory name, so I suggest dropping the “for” loop.

    Ok, here is a version without loop. I also added "-" before the if to mimick other lines of the bininstall target.

    Does it look better now?

    (...) there is technically still a race with “libainstall” and “bininstall” both testing and creating the same directory.

    Sorry, I don't understand. Is it an issue in practice to create the same directory twice "at the same time"? Are you able to reproduce the bug?

    Please try my use case: "./configure && make && make -j10 install" and enjoy the partial installation :-) (The python binary is installed in bin/ but it doesn't work because the stdlib is completly missing.)

    @vadmium
    Copy link
    Member

    vadmium commented Nov 28, 2015

    I agree your patches should fix your practical bug (I can reproduce it, two out of three times). My concern is more about making the code cleaner and less likely to grow problems in the future. :) The worse case with patch 3 would be the “Creating directory” message being echoed twice. I am just trying to get the makefile cleaner.

    If that block compiled a file, instead of just creating a directory, then it would be a worse problem. The second compile command could truncate the first version, causing errors later on.

    Also, I don’t believe adding the dash (-) prefix is a good idea here. It tells Make to keep going even if the command failed. In this case, if the directory already exists, the “if” statement will succeed, and the only way it can fail is if $(INSTALL) fails, which I think we would want to know about. The original “for” loop under libainstall does not include a dash.

    bininstall-4.patch implements my suggestion from before.

    @vstinner
    Copy link
    Member Author

    bininstall-4.patch: IMHO it's overkill and makes Makefile more complex to follow. My patch bininstall.patch proposed something similar, but Arfrever asked me to modify bininstall target. IMHO Getting "Creating directory xxx" message twice is a minor issue.

    It becomes annoying to have nitpicking on my patch, I hate autotools, I hate having to care of Makefile, etc. Maybe I should just abandon my change, it looks like nobody cares that Python cannot be installed with -j9 anyway :-)

    @vadmium
    Copy link
    Member

    vadmium commented Nov 29, 2015

    Please go ahead with bininstall-3.patch if you prefer. It certainly avoids the problem you reported. Although I still encourage you to not add the dash before the command.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 13, 2015

    New changeset c03ef448b5b2 by Victor Stinner in branch '2.7':
    Issue bpo-25696: Fix installation of Python on UNIX with make -j9.
    https://hg.python.org/cpython/rev/c03ef448b5b2

    New changeset 87d96b349ff5 by Victor Stinner in branch '3.5':
    Issue bpo-25696: Fix installation of Python on UNIX with make -j9.
    https://hg.python.org/cpython/rev/87d96b349ff5

    New changeset 8dd594d36704 by Victor Stinner in branch 'default':
    (Merge 3.5) Issue bpo-25696: Fix installation of Python on UNIX with make -j9.
    https://hg.python.org/cpython/rev/8dd594d36704

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 13, 2015

    New changeset 94910d210ef4 by Victor Stinner in branch '2.7':
    Issue bpo-25696: Don't ignore errors in 'make bininstall' on creating $(LIBPC) directory
    https://hg.python.org/cpython/rev/94910d210ef4

    New changeset d28268c47421 by Victor Stinner in branch '3.5':
    Issue bpo-25696: Don't ignore errors in 'make bininstall' on creating $(LIBPC) directory
    https://hg.python.org/cpython/rev/d28268c47421

    @vstinner
    Copy link
    Member Author

    Please go ahead with bininstall-3.patch if you prefer.

    Ok done.

    It certainly avoids the problem you reported. Although I still encourage you to not add the dash before the command.

    Oops, fixed.

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants