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

WARNING: preserve ownership not supported on platform #646

Closed
c9845 opened this issue Dec 11, 2023 · 11 comments
Closed

WARNING: preserve ownership not supported on platform #646

c9845 opened this issue Dec 11, 2023 · 11 comments

Comments

@c9845
Copy link

c9845 commented Dec 11, 2023

Over the last month or so, I have been receiving a 'WARNING: preserve ownership not supported on platform' warning when using the minify binary to compress files. I also get a resulting spike in CPU usage (the messages are logged constantly until the process is killed). I receive this message when minifying CSS or JS, using a command such as follows:
minify -w -v -o website/static/css/styles.min.css website/static/css/styles.css. Note, however, that minifying does work, most of the time, unless CPU is exhausted.

I cannot identify what the issue is, or why it started occurring. I have used minify in my development process for a long time without issue. I tried to do some debugging without success.

I am running W11 22H2 (this occurred on 22H1 too).
I installed minify using go install github.com/tdewolff/minify/v2/cmd/minify@latest.
I am on the latest version of minify.

@c9845
Copy link
Author

c9845 commented Dec 11, 2023

I just did some manual bisecting of releases to see (1) if this was related to a change in minify or Windows, and (2) to see if there was a specific minify version where this cropped up.

The issue starts occurring at minify version v2.19.10. It does not occur at v2.12.9.

I haven't diff-ed the versions yet, or dug into the code, to identify an issue in the code, but this clearly aligns with something in preserveAttributes().

@c9845
Copy link
Author

c9845 commented Dec 11, 2023

Reference: --preserve or -p flag to minify....

@c9845
Copy link
Author

c9845 commented Dec 11, 2023

It looks like the default --preserve value was changed, although I am not entirely sure.

In v2.12.9 is was nil. In v2.19.10 is was changed to []string{"mode", "ownership", "timestamps"}. However, in the non-warning-logging v2.12.9 there is a f.Lookup().NoOptDefVal that looks like it is applying a default value anyway. I am assuming this NoOptDefVal was broken and the default was just applied to the --preserve flag parsing?

Commit 38f0e88 removed the NoOptDefVal.

@tdewolff
Copy link
Owner

tdewolff commented Dec 11, 2023

Thanks for the bug report. There was a big change in library for the commit you reference, from using spf13's pflag to my own argp. Before, the default value when not supplied was nil which was then set to mode, ownership, timestamps using the NoOptDefVal you mention, and that should still be the case. So I'm not sure why this behaviour changed for you...!

You can remove the -v flag to prevent logging warnings. Are you minifying many files? That would explain the CPU spike, I think we should log warnings only once (fixed in 729b3d1). Otherwise, the fix would be to use --preserve=[mode timestamps] since preserving ownership on Window sis not supported (yet).

tdewolff added a commit that referenced this issue Dec 11, 2023
@tdewolff
Copy link
Owner

Changed that if getting/setting ownership is not supported on platform, the default value for --preserve is now mode,timestamps

@c9845
Copy link
Author

c9845 commented Dec 11, 2023

I saw that the previous setting for --preserve was nil which then got set to mode, ownership, timestamps but for some reason this error does not occur. It have a feeling nil is being used instead of the NoOptDefVal. Regardless, that is in an old version of this....

I did start using the --preserve flag so that solves my issue! The new functionality to automatically determine if "ownership" is supported is even better! Much appreciated.


I am not minifying a lot of files. One .css to .min.css. My CPU just sits at 25%+ for the minify process. I am not sure what it is doing!

@tdewolff
Copy link
Owner

tdewolff commented Dec 11, 2023 via email

@tdewolff
Copy link
Owner

I'm pretty sure the problem is a little further down in the function, where I compare the root path with the src path, and unless they are equal it goto the beginning. In Windows the paths never match due to the forward backward slash difference.

No need to send me the file thus, let me work on a fix and I'll need you to verify it's working, thanks!

@c9845
Copy link
Author

c9845 commented Dec 12, 2023

Taco,
I am going to open a new issue for the high CPU usage to keep the issues more singular focused...

Plus, it looks like the "preserve ownership" sort-of-a-bug has been fixed.

@tdewolff
Copy link
Owner

Alright, in the mean time I've pushed out a (possible) fix for the endless loop. Are you able to test from the master branch? You need to clone the repository and run make install in the repo directory (or othersider go to cmd/minify/ and run go install). Thanks!

@c9845
Copy link
Author

c9845 commented Dec 12, 2023

I cloned the repo and tested. The --preserve issue is fixed!

kodiakhq bot referenced this issue in cloudquery/codegen Jan 1, 2024
)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/tdewolff/minify/v2](https://togithub.com/tdewolff/minify) | indirect | patch | `v2.20.6` -> `v2.20.10` |

---

### Release Notes

<details>
<summary>tdewolff/minify (github.com/tdewolff/minify/v2)</summary>

### [`v2.20.10`](https://togithub.com/tdewolff/minify/releases/tag/v2.20.10)

[Compare Source](https://togithub.com/tdewolff/minify/compare/v2.20.9...v2.20.10)

-   [cmd: default value for --preserve only includes ownership if supported on platform](https://togithub.com/tdewolff/minify/commit/d579304e76fbf4c9de4b6cafab3dd9246c00edcc), see [https://github.com/tdewolff/minify/issues/646](https://togithub.com/tdewolff/minify/issues/646)
-   [cmd: fix endless loop on preserving attributes on Windows, see](https://togithub.com/tdewolff/minify/commit/ca97b30acea0a22471011ccd3b075325a5e576b8) [https://github.com/tdewolff/minify/issues/646](https://togithub.com/tdewolff/minify/issues/646)
-   Fix various parsing bugs for JS/HTML in `tdewolff/parse`

### [`v2.20.9`](https://togithub.com/tdewolff/minify/releases/tag/v2.20.9)

[Compare Source](https://togithub.com/tdewolff/minify/compare/v2.20.8...v2.20.9)

Fix build

### [`v2.20.8`](https://togithub.com/tdewolff/minify/releases/tag/v2.20.8)

[Compare Source](https://togithub.com/tdewolff/minify/compare/v2.20.7...v2.20.8)

-   [HTML: fix parsing of PHP/ASP/EJS files](https://togithub.com/tdewolff/minify/commit/5b4030cc8240edf4b87bc8d195bc3f3d1ca46f50)
-   [cmd: support ASP, PHP, Mustache, Handlebars, EJS and Go templates for command line](https://togithub.com/tdewolff/minify/commit/5b4030cc8240edf4b87bc8d195bc3f3d1ca46f50)

### [`v2.20.7`](https://togithub.com/tdewolff/minify/releases/tag/v2.20.7)

[Compare Source](https://togithub.com/tdewolff/minify/compare/v2.20.6...v2.20.7)

-   [JS: fix parsing of empty for-loop,](https://togithub.com/tdewolff/minify/commit/be084bb11b53852ae0b8147585b210bed1f85a85) [fixes](https://togithub.com/tdewolff/minify/commit/be084bb11b53852ae0b8147585b210bed1f85a85) [https://github.com/tdewolff/minify/issues/636](https://togithub.com/tdewolff/minify/issues/636)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am on the first day of the month" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMTUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjExNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants