ntindle commented 2 weeks ago

Background

The current bdist_dmg command is limited in its functionality, to mainly the

naming the dmg
adding an app shortcut
doing it quietly

Changes

This PR

Moves bdist_dmg out of bdist_mac
Adds support for dmgbuild and its features such as backgrounds and layout customization

Files	Patch %	Lines
cx_Freeze/command/bdist_dmg.py	94.38%	10 Missing :warning:

For context, before we start discussing them, dmgbuild encourages a settings.py file similar to the setup.py cx_freeze uses. Here is one of the settings.py files. You can also use a settings.json. The downside to all of these options is there's not really a bindable interface for any of them unfortunately

Make a settings.py file dynamically based on passed-in variables
configure things via the command line
Have the user make a settings.py and use that in combination with setup.py
Use a settings.json settings file we build live
call builddmg.core.build_dmg() directly, with parameters built from user_options in cx_Freeze

My intent is to try 5 -> 4 -> 1 -> 3 -> 2 and update as needed. Let me know if you have any opinions

marcelotduarte commented 2 weeks ago

I see dmbuild has a lot of options. Using settings.py has some advantages, such as using the Info.plist generated by bdist_mac (from what I saw on example), but it requires using another configuration file. We can think about how bdist_rpm was made. Rpm needs a .spec file and this file is generated by the options that the user passes, so the user does not need to deal with the .spec format. On the other hand, cx_Freeze allows the user to use setup.py, command line and pyproject.toml, all directing the options to the chosen build/bdist command. You can use a template to generate settings.py on the fly from the available options (which is probably an extension of what you thought of in 1). The way cx_Freeze is done, based on setuptools, you will need to think about the command line, and the setup.py and pyproject.toml options will automatically be available.

I believe that the finalize_options function can be built based on the load_json function.

ntindle commented 2 weeks ago

For some of these options, like icon: https://dmgbuild.readthedocs.io/en/latest/settings.html#content-settings

Is this something better pulled from the bdist_mac config or should they be exposed here?

Current output:

 python setup.py bdist_dmg --help
Common commands: (see '--help-commands' for more)

  setup.py build      will build the package underneath 'build/'
  setup.py install    will install the package

Global options:
  --verbose (-v)  run verbosely (default)
  --quiet (-q)    run quietly (turns verbosity off)
  --dry-run (-n)  don't actually do anything
  --help (-h)     show detailed help message
  --no-user-cfg   ignore pydistutils.cfg in your home directory

Options for 'bdist_dmg' command:
  --volume-label           Volume label of the DMG disk image
  --applications-shortcut  Boolean for whether to include shortcut to
                           Applications in the DMG disk image
  --silent (-s)            suppress all output except warnings
  --format                 format of the disk image (default: UDZO)
  --filesystem             filesystem of the disk image (default: HFS+)
  --size                   If defined, specifies the size of the filesystem
                           within the image. If this is not defined, cx_Freeze
                           (and then dmgbuild) will attempt to determine a
                           reasonable size for the image. If you set this, you
                           should set it large enough to hold the files you
                           intend to copy into the image. The syntax is the
                           same as for the -size argument to hdiutil, i.e. you
                           can use the suffixes `b`, `k`, `m`, `g`, `t`, `p`
                           and `e` for bytes, kilobytes, megabytes, gigabytes,
                           terabytes, exabytes and petabytes respectively.
  --background (-b)        A rgb color in the form #3344ff, svg named color
                           like goldenrod, a path to an image, or the words
                           'builtin-arrow'
  --show-status-bar        Show the status bar in the Finder window. Default
                           is False.
  --show-tab-view          Show the tab view in the Finder window. Default is
                           False.
  --show-path-bar          Show the path bar in the Finder window. Default is
                           False.
  --show-sidebar           Show the sidebar in the Finder window. Default is
                           False.
  --sidebar-width          Width of the sidebar in the Finder window. Default
                           is None.
  --window-rect            Window rectangle in the form x,y,width,heightThe
                           position of the window in ((x, y), (w, h)) format,
                           with y co-ordinates running from bottom to top. The
                           Finder makes sure that the window will be on the
                           user's display, so if you want your window at the
                           top left of the display you could use (0, 100000)
                           as the x, y co-ordinates. Unfortunately it doesn't
                           appear to be possible to position the window
                           relative to the top left or relative to the centre
                           of the user's screen.
  --icon-locations         A dictionary specifying the co-ordinates of items
                           in the root directory of the disk image, where the
                           keys are filenames and the values are (x, y)
                           tuples. e.g.:icon-locations = { "Applications":
                           (100, 100), "README.txt": (200, 100) }
  --default-view           The default view of the Finder window. Possible
                           values are "icon-view", "list-view", "column-view",
                           "coverflow".
  --show-icon-preview      Show icon preview in the Finder window. Default is
                           False.
  --license                Dictionary specifying license details with 'default
                           -language', 'licenses', and 'buttons'.default-
                           language: Language code (e.g., 'en_US') if no
                           matching system language.licenses: Map of language
                           codes to license file paths (e.g., {'en_US':
                           'path/to/license_en.txt'}).buttons: Map of language
                           codes to UI strings ([language, agree, disagree,
                           print, save, instruction]).Example: {'default-
                           language': 'en_US', 'licenses': {'en_US':
                           'path/to/license_en.txt'}, 'buttons': {'en_US':
                           ['English', 'Agree', 'Disagree', 'Print', 'Save',
                           'Instruction text']}}

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

marcelotduarte commented 2 weeks ago

For some of these options, like icon: https://dmgbuild.readthedocs.io/en/latest/settings.html#content-settings

Is this something better pulled from the bdist_mac config or should they be exposed here?

It must be pulled from the bdist_mac configuration, but sometimes if the user does not realize this inheritance, it can cause confusion. So if possible inherit the configuration, but also expose it. In the specific case of the icon, in bdist_appimage I use the icon specified in the Executable class, and if not, a default is used. In bdist_mac, this is not done (I think the icon is ignored). Maybe we should fix it to standardize.

ntindle commented 2 weeks ago

I’ll reuse the pattern from app image in that case

ntindle commented 2 weeks ago

Still working on this, don’t worry :)

ntindle commented 2 weeks ago

Is there a way to get Ruff to stop complaining about the commented-out code in dmg/setup.py and the type issue on self.distribution.executables in bdist_dmg.py?

Also can we change the CI to not always fail on failing to upload to codecov

ntindle commented 2 weeks ago

Current status btw

marcelotduarte commented 2 weeks ago

To ignore all violations across an entire file, add the line # ruff: noqa anywhere in the file, preferably towards the top, like so: # ruff: noqa

Also can we change the CI to not always fail on failing to upload to codecov

# pragma: no cover

But, the real fail is: https://github.com/marcelotduarte/cx_Freeze/actions/runs/9473801199/job/26102156665?pr=2442#step:6:436

marcelotduarte commented 2 weeks ago

Current status btw

It's getting really good, huh?

ntindle commented 2 weeks ago

This one is probably a better example of what I was talking about. There’s likely failures in that last commit because it was my end of day commit. It triggered the ruff comment because it wouldn’t let me commit to the pr with issues without doing —no-verify

https://github.com/marcelotduarte/cx_Freeze/actions/runs/9472361645/job/26097601102

marcelotduarte commented 2 weeks ago

Earlier I modified a test exactly to see if it passed the tests. It was just to see if they passed the test. But you should ignore the comments for now. There is also the option of putting it at the end of the commit message [ci skip]

ntindle commented 1 week ago

Updated the code to pass the tests. It looks like the failure now is the codecov lacking a token. I'll see if that's something I can run in my fork. https://github.com/marcelotduarte/cx_Freeze/actions/runs/9491540029/job/26157164775?pr=2442#step:7:63

ntindle commented 1 week ago

After this, I'll be working on licensing display for MSI if you have any resources to share 😄

marcelotduarte commented 1 week ago

It looks like the failure now is the codecov lacking a token. I'll see if that's something I can run in my fork.

The codecov/coverage is very interesting as it helps to resolve some errors. But it has this error, when a contributor makes more than one commit (apparently that's it), it gives this error, and I have to change the token.

marcelotduarte commented 1 week ago

After a rebase, the tests pass, including coverage.

ntindle commented 1 week ago

Awesome! Let me know any changes required to get merged, and I'll keep an eye on this 😄

marcelotduarte commented 1 week ago

I'll let you know later. In another thread, you asked about things you can look at. For macOS, there are some issues that I can't check. Do you use any GUI (pyqt5/6, pyside2/6, etc)? Do you need to notarize, sign the executables? Maybe you can look at some of these issues: 2212, 2180, 1228.

ntindle commented 1 week ago

I can work on the changes requested here in a few hours. We currently don’t use a GUI and if we did so it may be a web ui. Not sure for now.

We will be doing code signing but I don’t have any certs on hand to do so yet

ntindle commented 1 week ago

Mentioned Issues for review: We likely will take a look in a few months as we add internationalization to our system #2212, I will be looking into #2180 when I get our signing certs from Apple, As I understand, this behavior is expected on macOS and is the reason for PR #1228.

marcelotduarte commented 4 days ago

I hope you don't mind, I will correct the errors reported by the pre-commit.

ntindle commented 4 days ago

Always feel free to. My general opinion is if people don’t want maintainers to push to their PRs they should uncheck the box to allow it

marcelotduarte commented 4 days ago

At least it formatted the table. https://cx-freeze--2442.org.readthedocs.build/en/2442/bdist_dmg.html

marcelotduarte commented 1 hour ago

It's excellent. I'm going to do some tests (at least on Windows, I don't know if I'll be able to do it on Mac because I use a VM) and I should merge it with other PRs I'm working on. Do you need version 7.2 with its PRs to be released now (in a few days) or can you wait a little?

ntindle commented 1 hour ago

I'll be going on vacation tomorrow, but I've already got tests working off my fork for our CI, so it shouldn't be a problem to wait. I would encourage more testing. I'll be back July 10 and can pick up from there anything needed

marcelotduarte / cx_Freeze

Add dmgbuild as a dependency to improve mac dmg #2442

Background

Changes

Related

Codecov Report