emacscollective / borg

Assimilate Emacs packages as Git submodules
https://emacsmirror.net/manual/borg
GNU General Public License v3.0
255 stars 28 forks source link

borg-assimilate a new package will change the encoding of .gitmodules on Windows #128

Closed Eason0210 closed 2 years ago

Eason0210 commented 2 years ago

Hi @tarsius,

Recently, Borg works well on Linux and macOS, But When I transfer my .emacs.dto Windows system, It works well after make bootstrap . But when I add new packages by borg-assimilate, It will change the encoding of .gitmodules to unix style.

Before borg-assimilate <package name>, C-h f buffer-file-coding-system, will get this:

buffer-file-coding-system is a variable defined in ‘C source code’.
Its value is ‘undecided-dos’
Local in buffer .gitmodules; global value is utf-8

After borg-assimilate <package name>, C-h f buffer-file-coding-system, will get this:

buffer-file-coding-system is a variable defined in ‘C source code’.

Its value is ‘undecided-unix’
Local in buffer .gitmodules; global value is utf-8

And there will showing many ^M at the end of lines. I think it is becasue the file is CRLFstyle instead ofLFstyle. image

It would be nice if borg-assimilate can repect the current encoding of .gitmodules when writing new content.

And I guess it is this commit https://github.com/emacscollective/borg/commit/dee07ac, which introduce the issue

tarsius commented 2 years ago

And I guess it is this commit https://github.com/emacscollective/borg/commit/dee07ac, which introduce the issue

This code was copied from package-autoload-ensure-default-file and package-generate-autoloads. Also coding-system-for-write is only let-bound around the call to write-region. If this binding leaks into the values of related variables in other buffers, then that likely is a bug in Emacs. Do you use 29.0.50?

Try wrapping this code in with-temp-buffer.

Eason0210 commented 2 years ago

If this binding leaks into the values of related variables in other buffers, then that likely is a bug in Emacs. Do you use 29.0.50?

Yes I am using Emacs 29.0.50 at this commit: https://github.com/emacs-mirror/emacs/commit/aab560f0c1955bae57cc35a71be95b5bfa2ab525

Try wrapping this code inwith-temp-buffer

I will try to dig deeper

By the way, I just notice that all the <package-name>-autoloads.el aslo the many ^M

Eason0210 commented 2 years ago

Try wrapping this code in with-temp-buffer.

I tried to change https://github.com/emacscollective/borg/blob/2b962729336a53ada638215485913bfd75fc7102/borg.el#L726-L728
as bellow, and it works well:

(with-temp-buffer
        (let ((coding-system-for-write 'utf-8-emacs-unix))
          (write-region (autoload-rubric file "package" nil)
                        nil file nil 'silent)))

Borg-assimilate a new package will not change the encoding of the .gitmodules , and the new created <package-name>-autoloads.el aslo will not show ^M, it's buffer-file-coding-system is utf-8-emacs-unix, is this expected?

tarsius commented 2 years ago

At some point I'll have to investigate more but for now I have just added that kludge.

Eason0210 commented 2 years ago

If this binding leaks into the values of related variables in other buffers, then that likely is a bug in Emacs. Do you use 29.0.50?

Hi @tarsius , You are right, may be it is a bug of Emacs 29.0.50. I tried to use Emacs 28.1, and It works well before this commit https://github.com/emacscollective/borg/commit/9917084fb806796f91025dbec9f946f4415059ed.

But I have no idear how to reproduce this issue withemacs -q, do you have any advice? If I can reproduce it, I will report it to Emacs upstream.

tarsius commented 2 years ago

I cannot reproduce it but maybe you can with:

(let ((file "/tmp/autoload"))
  (with-temp-buffer
    (setq buffer-file-coding-system 'undecided-dos)
    (message "A %s" buffer-file-coding-system)
    (let ((coding-system-for-write 'utf-8-emacs-unix))
      (write-region (autoload-rubric file "package" nil)
                    nil file nil 'silent))
    (message "B %s" buffer-file-coding-system)))

Otherwise start with borg-update-autoloads and remove the bits that depend on the rest of borg. Hopefully you can still reproduce now. Then keep removing things step by step while trying if the issue goes away after each step, slowly turning that function into the code above.

Eason0210 commented 2 years ago

I cannot reproduce it but maybe you can with:

(let ((file "/tmp/autoload"))
  (with-temp-buffer
    (setq buffer-file-coding-system 'undecided-dos)
    (message "A %s" buffer-file-coding-system)
    (let ((coding-system-for-write 'utf-8-emacs-unix))
      (write-region (autoload-rubric file "package" nil)
                    nil file nil 'silent))
    (message "B %s" buffer-file-coding-system)))

Unfortunately, I couldn't reproduce the issue with this code either.

I tried to use Emacs 28.1, and It works well before this commit https://github.com/emacscollective/borg/commit/9917084fb806796f91025dbec9f946f4415059ed.

With double check. The issue also happeneded on Emacs 28.1.

I tried to change line 726 inborg.el as bellow:

(with-temp-buffer ; Kludge for #128.
        (message "A %s" buffer-file-coding-system)
        (let ((coding-system-for-write 'utf-8-emacs-unix))
          (write-region (autoload-rubric file "package" nil)
                        nil file nil 'silent)
          (message "B %s" buffer-file-coding-system)))

And then M-x borg-assimilate RET vundo, will get these message:

(09:38:16) Building vundo

Initializing drones...done (59 drones in 0.399s)
 Creating c:/Users/Eason/.emacs.d/lib/vundo/vundo-autoloads.el...
A chinese-gbk-dos
B chinese-gbk-dos
Compiling c:/Users/Eason/.emacs.d/lib/vundo/vundo.el...
Done (Total of 1 file compiled)

Process emacs ... --eval (borg-build "vundo") finished