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

emacs-app-devel: update to 20241021 #26002

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tsujp
Copy link
Contributor

@tsujp tsujp commented Sep 29, 2024

Description

This is a weird one I think since I have chosen to specifically only update emacs-app-devel which also has differing compilation flags (native compilation is now default for example).

As and when Emacs 30 gets released I imagine the Portfile will get simpler, I've been using this general structure since August rebuilding from Emacs' master every other week or so.

Tested on

macOS 14.5 23F79 arm64
Command Line Tools 15.3.0.0.1.1708646388

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@macportsbot
Copy link

Notifying maintainers:
@catap for port emacs.
@drkp for port emacs.

@macportsbot macportsbot added type: update maintainer: open Affects an openmaintainer port labels Sep 29, 2024
editors/emacs/Portfile Show resolved Hide resolved
epoch 5
version 20240804
github.setup emacs-mirror emacs a06a7209028363a1bb2f727ffaecdf4d02296b2e
epoch 1
Copy link
Contributor

Choose a reason for hiding this comment

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

the epoch field can never decrease, it should stay at 5 (see the guide)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I really should try find my MacPorts notes again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to bump it to 6 so it guaranteed invalidates prior versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

no, don't do that. You updates the version so there is absolutely no need to do so. The only reason to increase the "epoch" is when you want to downgrade the version or of the versioning has changed so that a newer version isn't recognized as newer by "vercomp".

Please refer to the guide entry O linked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing so just in-case as I had a brain fart and thought it also compared the commit hash (late and very tired). I did read the link you sent before acting hehe. I'll set it back to 5.

editors/emacs/Portfile Outdated Show resolved Hide resolved
editors/emacs/Portfile Show resolved Hide resolved
depends_lib-delete port:jansson
configure.args-append --with-native-compilation=no

# TODO: Put this somewhere better. See https://github.com/emacs-mirror/emacs/commit/4c6f45fa8eef1a15d5790c1f3d3e608b548015db#diff-731c852b7c27c6ee90c44c871e2988ecfc2d225bc720156d7a47c1d563c2683aR59
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the purpose of this comments; what would be a better place to put it? And if there is such a place why not do it now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes. Do you want this as a new variant option or always present by default? The linked comment:

** New configuration option '--disable-gc-mark-trace'.
This disables the GC mark trace buffer for about 5 % better garbage
collection performance.  Doing so may make it more difficult for Emacs
developers to help finding GC-related bugs that you run into, which is
why it the mark trace buffer is enabled by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

well, it depends - do you think many users will be "Emacs developers" as that seems to be the usual situation where you might not want to have it disabled. I would probably have it use this configure.arg always; if @drkp has a different opinion he can always change it and/or put it in a variant.

@reneeotten reneeotten requested a review from drkp October 20, 2024 01:21
@tsujp tsujp requested a review from reneeotten October 20, 2024 17:50
@tsujp
Copy link
Contributor Author

tsujp commented Oct 21, 2024

@reneeotten I've updated the devel version to 20241021 master HEAD b3af195213518514f78ac6f66f9598e45befd1ec.

@tsujp tsujp changed the title emacs-app-devel: update to 20240926 emacs-app-devel: update to 20241021 Oct 21, 2024
if {[variant_isset nativecomp]} {
notes "emacs devel subports don't always keep compatibility for native\
compiled files. Better to cleanup your ~/.emacs.d/.local/cache/eln"

configure.args-delete --with-native-compilation=no
configure.args-append --with-native-compilation=aot
Copy link
Contributor

Choose a reason for hiding this comment

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

this could/should use configure.args-replace instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will get to it, going to wait for @drkp's feedback for the other item in this file (do both at once).

Copy link
Contributor

@reneeotten reneeotten Oct 23, 2024

Choose a reason for hiding this comment

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

Just finish it up. None of the maintainers has responded in over three weeks, so don't wait. Just get these last things finalized and I'll merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alrighty, will get on it in an hour or two.

depends_lib-delete port:jansson
configure.args-append --with-native-compilation=no

# TODO: Put this somewhere better. See https://github.com/emacs-mirror/emacs/commit/4c6f45fa8eef1a15d5790c1f3d3e608b548015db#diff-731c852b7c27c6ee90c44c871e2988ecfc2d225bc720156d7a47c1d563c2683aR59
Copy link
Contributor

Choose a reason for hiding this comment

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

well, it depends - do you think many users will be "Emacs developers" as that seems to be the usual situation where you might not want to have it disabled. I would probably have it use this configure.arg always; if @drkp has a different opinion he can always change it and/or put it in a variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: update
Development

Successfully merging this pull request may close these issues.

4 participants