Re: New "make benchmark" target

[Pip Cet]

"Andrea Corallo" <acorallo@gnu.org> writes:

> Pip Cet <pipcet@protonmail.com> writes:
>
>> "Andrea Corallo" <acorallo@gnu.org> writes:
>>
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>
>>>>> From: Stefan Kangas <stefankangas@gmail.com>
>>>>> Date: Wed, 15 Jan 2025 14:20:42 -0800
>>>>> Cc: acorallo@gnu.org, mattiase@acm.org, eggert@cs.ucla.edu,
>>>>>   emacs-devel@gnu.org
>>>>>
>>>>> Pip Cet <pipcet@protonmail.com> writes:
>>>>>
>>>>> > "Eli Zaretskii" <eliz@gnu.org> writes:
>>>>> >
>>>>> >>> I'm happy to change the scratch/elisp-benchmarks branch in the ways
>>>>> >>> we've discussed, and it should be merged, but if someone decides to
>>>>> >>> incrementally solve some of the issues, that, while not very harmful,
>>>>> >>> would be an inefficient use of resources.
>>>>> >>
>>>>> >> Let's hear what Andrea thinks about the issues you reported (let's
>>>>> >> please discuss them on the bug tracker, not here), and let's take it
>>>>> >> from there.
>>>>> >
>>>>> > Sure, just let me know when we can merge the branch (assuming you meant
>>>>> > to say that the merge, too, should wait for Andrea's thoughts) or which
>>>>> > other changes are required!
>>>>>
>>>>> Is there anything left to do here that is blocking the merge?
>>>>
>>>> I didn't follow the discussions too closely, but if there are no
>>>> unresolved issues left between Pip and Andrea, I think this can land.
>>>>
>>>> Andrea, any objections or comments?
>>>
>>> I've no objections, only two questions:
>>
>> Thanks!  I'll merge without the admin/elpa2emacs.sh script (which the
>> questions are about), then?
>
> Hi Pip,
>
> I'd prefer we finalize the workflow first, otherwise I don't know how to
> merge in emacs.git the fixes/improvements we make in ELPA.

Okay!

>
>>> 1- admin/elpa2emacs.sh looks elisp-benchmarks specific, if that's the
>>>    case perhaps should have a better name?  Otherwise we should probably
>>>    make it general?
>>
>> You're right, it currently hardcodes the "benchmarks/" destination
>> directory.  My reasoning for this was that other elpa modules would most
>> likely require different files to be put into different directories, and
>> so the script would need modifying anyway.  (As git filter-repo isn't
>> exactly well-behaved, it would probably be easiest to run the script
>> once for each destination directory).
>>
>>> 2- What is the workflow for importing into emacs.git existing changes in
>>>    elisp-benchmarks?  I tried to run elpa2emacs.sh as suggested but I do
>>>    get an error (not sure if the suggested invocation is for the initial
>>>    import, for merging incremental changes or both).
>>
>> What kind of error?  There are two problems here:
>>
>> 1. we add a remote called elpa2emacs-filtered-elpa, but we don't remove
>> it automatically.  Re-running elpa2emacs.sh may use the old remote, with
>> undesirable results.  But removing a remote means interfering with an
>> existing git repository even more (I destroyed several emacs repos while
>> adjusting this script).
>
> Okay but I guess we can assume elpa2emacs-filtered-elpa is a name that
> would not be choosen by a developer no?  Otherwise IMO we have to add in
> the comment instructions on how to re-run elpa2emacs.sh.

You're right.  I'll add the code to remove the remote again.

>> 2. we hardcode origin/master as the Emacs branch we're working on.  I
>> believe this is forgivable, but it means we can't currently test the
>> merge case, because origin/master doesn't contain the elisp benchmarks
>> yet.
>
> I've git.sv.gnu.org/srv/git/emacs.git called 'savannah' instead of
> 'origin' as I've other remotes.  I think we should have a way to specify
> the remote name.

Hmm.  Wouldn't that be confusing in the case where people use a fresh
checkout rather than a new worktree?

Let me think about it: I think it might be best to write the script so
it's run in an existing repo, adds the worktree itself, then removes it
when it's done.

> Also I think the script should stop if any of the commands fails while I
> see now it keep on running.

Agreed.

Would it be okay to make this script include a safety prompt?  It
modifies the git repository in potentially destructive ways,
particularly if we change it to be run in the main emacs repo...

Pip