Skip to content

Fix multipackage#4303

Merged
htgoebel merged 6 commits intopyinstaller:developfrom
coreydexter:multi_package_fix
Apr 9, 2020
Merged

Fix multipackage#4303
htgoebel merged 6 commits intopyinstaller:developfrom
coreydexter:multi_package_fix

Conversation

@coreydexter
Copy link
Copy Markdown
Contributor

@coreydexter coreydexter commented Jul 7, 2019 •
edited
Loading

Fix up the multipackage so that exe's can shared dependencies. Closes #1527

There are a few changes that needed to be done:

  • Move the multipackage tests from old_suite to the new py.test suite.
  • MERGE class: Correct the relative paths which reference shared dependencies from one exe to another
  • Bootloader: Fix a bug in the pyi_copy_file where when files were copied, an extra bit of data was been added if the file didn't finish perfectly on the buffer size. fwrite writes the entire buffer, not just up until a null terminator character, so this meant the last fwrite would include data from the previous fwrite which corrupted files.
  • Bootloader: Fix a bug where an archive sometimes wouldn't be opened because malloc doesn't NULL values on initialisation.

@coreydexter coreydexter changed the title 1527 - Fix multipackage Fix multipackage Jul 7, 2019
@coreydexter coreydexter mentioned this pull request Jul 7, 2019
Closed
@coreydexter coreydexter force-pushed the multi_package_fix branch 3 times, most recently from ea5af32 to 52c2df9 Compare July 8, 2019 11:30
@htgoebel htgoebel mentioned this pull request Aug 7, 2019
Closed
@coreydexter

This comment has been minimized.

Sign in to view
@papr

This comment has been minimized.

Sign in to view
@ErikBjare

This comment has been minimized.

Sign in to view
htgoebel
htgoebel suggested changes Nov 25, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@htgoebel htgoebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this pull-request.I now found time for reviewing it and added some inline-remarks.

Beside these:

  • Please reword the commit message to follow our Commit Message Rules (esp. the prefix and to explain a bit more what and why this has been done. You did this already in the pull-request message ;-) For us the commit message is much more important.
  • Please split the news-fragment and put it into the resp. commit.

(git rebase -i and git push -f are your friends)

I will then merge this as a branch with an appropriate commit message containing the issue number.

@coreydexter coreydexter force-pushed the multi_package_fix branch 3 times, most recently from 0060fea to 47b8bcb Compare November 29, 2019 08:34
@coreydexter coreydexter requested a review from htgoebel December 4, 2019 01:00
@sfgeorge
Copy link
Copy Markdown

sfgeorge commented Jan 5, 2020

@htgoebel Just checking, can you re-review this when you get a chance? Thanks so much!

@Legorooj Legorooj added @high area:bootloader Caused be or effecting the bootloader bug feature Feature request labels Feb 8, 2020
@kmcquade
Copy link
Copy Markdown

kmcquade commented Mar 2, 2020

@htgoebel, any chance you can re-review, as requested by @sfgeorge above? The parent issue for this (#1527) has been open for almost 5 years now :)

@Legorooj
Copy link
Copy Markdown
Member

Legorooj commented Mar 16, 2020

@htgoebel

@Legorooj Legorooj added this to the PyInstaller 4.0 milestone Mar 16, 2020
@Legorooj Legorooj mentioned this pull request Mar 16, 2020
Closed
@htgoebel htgoebel force-pushed the multi_package_fix branch from 47b8bcb to 426de47 Compare April 8, 2020 19:50
@coreydexter @htgoebel
Tests: Move multipackage tests to py.test suite
0fefd1f
@coreydexter @htgoebel
building: Ensure DEPENDENCY's correctly get MERGE'd
f177fdf 
The common_prefix has already been normcase'd, so ensure that
the path we are trying to remove the common_prefix from is
also normcase'd so that replace can work.

When defining name of the dependency get the basename from
tpl[1] rather than using tpl[0] as tpl[0] may have not have
the proper casing (it may have been normcase'd) which will
cause issues on operating systems where filename are
case-sensitive (eg Linux).
@coreydexter @htgoebel
building: Ensure DEPENDENCY's are added to PKG TOC
3391d0c 
When a PKG is created the DEPENDENCY's need to be
added to ensure the resulting archive has a reference
to the relative file path of the DEPENDENCY and
hence the bootloader will be able to load it.
@coreydexter @htgoebel
Bootloader: Don't write stale data copying a file
b35f8bd 
Fix bug in pyi_copy_file where when files were copied, an extra
bit of data was being added if the file didn't finish perfectly
on the buffer size. fwrite writes the entire buffer, not just
up until a null terminator character, so this meant the last
fwrite would include data from the previous fwrite which
corrupted files.
coreydexter and others added 2 commits April 8, 2020 23:32
@coreydexter @htgoebel
Bootloader: Ensure new archives get opened.
82f7510 
Use pyi_arch_status_new() (which uses calloc) to ensure archive->fp is NULL.
Otherwise the value could be non-NULL in which case the archive will not be
opened and bogus data will remain in archive->fp.
@htgoebel
bootloader: Use pyi_free_arch_status at another place.
3f4d5cf 
I missed this in 671770f.
@htgoebel htgoebel force-pushed the multi_package_fix branch from 426de47 to 3f4d5cf Compare April 8, 2020 21:32
@coreydexter coreydexter force-pushed the multi_package_fix branch 3 times, most recently from 96ca787 to ae85dee Compare April 9, 2020 02:21
@htgoebel
Copy link
Copy Markdown
Member

htgoebel commented Apr 9, 2020

@coreydexter I intentionally removed these "noqa: F401" changes :-)

@htgoebel
Copy link
Copy Markdown
Member

htgoebel commented Apr 9, 2020

@coreydexter I should have reasoned this: You added these comments to make the lint tests pass, wich basically is okay. But this is only test-code. And I count the information "renamed unchanged" more important than these errors. (Okay, what you most propably do not know: we only lint changed lines. So unless these lines will be touched again somewhen, lint will pass.)

@htgoebel htgoebel force-pushed the multi_package_fix branch from ae85dee to 3f4d5cf Compare April 9, 2020 18:32
@htgoebel htgoebel merged commit b3dd91c into pyinstaller:develop Apr 9, 2020
@htgoebel
Copy link
Copy Markdown
Member

htgoebel commented Apr 9, 2020

@coreydexter So many thanks for solving this long-standing issue!

@Legorooj
Copy link
Copy Markdown
Member

Legorooj commented Apr 10, 2020

@htgoebel the docs need fixing. They still say it's broken.

@simaoafonso-pwt
Copy link
Copy Markdown

simaoafonso-pwt commented Dec 21, 2021

@htgoebel the docs need fixing. They still say it's broken.

Besides this, is there any way to automatically merge several spec files?

@Legorooj
Copy link
Copy Markdown
Member

Legorooj commented Dec 21, 2021

Please open an issue if you have questions related to multipackage.

@pyinstaller pyinstaller locked as resolved and limited conversation to collaborators Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@htgoebel htgoebel htgoebel approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@htgoebel htgoebel

Labels

area:bootloader Caused be or effecting the bootloader bug feature Feature request @high

Projects

None yet

Milestone

PyInstaller 4.0

Development

Successfully merging this pull request may close these issues.

multipackage (MERGE) boken in PyInstaller 3.0

8 participants

@coreydexter @papr @ErikBjare @sfgeorge @kmcquade @Legorooj @htgoebel @simaoafonso-pwt