Skip to content

no-bug: A few quality-of-life tweaks (build tooling)#13565

Merged
mr-cheffy merged 1 commit into
zen-browser:devfrom
iskunk:pr-iskunk
May 6, 2026
Merged

no-bug: A few quality-of-life tweaks (build tooling)#13565
mr-cheffy merged 1 commit into
zen-browser:devfrom
iskunk:pr-iskunk

Conversation

@iskunk
Copy link
Copy Markdown
Contributor

@iskunk iskunk commented May 5, 2026

Hello, I am using some of the Zen build tools in an alternate setting, a framework that "converts" a Debian source package of Firefox into the equivalent Zen one. To that end, having some additional flexibility in the invocation of the tooling is helpful.


package.json: Use the $CARGO environment variable if set, and pass arguments to ffprefs using the new convention (see below)

(E.g. on Ubuntu, there is generally not a cargo executable in PATH, but there may be a cargo-1.91. This is normally specified via the CARGO variable, similar to CC and CXX)

update_service_dumps.py: Allow specifying DUMPS_FOLDER and ENGINE_DUMPS_FOLDER on the command line

ffprefs/src/main.rs: Allow specifying the prefs and engine dirs on the command line instead of hard-coding their locations relative to a common root dir

@iskunk iskunk requested a review from mr-cheffy as a code owner May 5, 2026 06:05
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. Improvement labels May 5, 2026
Copy link
Copy Markdown
Member

@mr-cheffy mr-cheffy left a comment

Choose a reason for hiding this comment

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

Thanks!

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 5, 2026
@mr-cheffy mr-cheffy requested a review from Copilot May 5, 2026 06:35

This comment was marked as resolved.

Comment thread tools/ffprefs/src/main.rs
}
}

fn is_twilight_build() -> bool {
Copy link
Copy Markdown
Member

@mr-cheffy mr-cheffy May 5, 2026

Choose a reason for hiding this comment

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

Is this being used correctly? As the DIR might be different?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, I missed that relative-to-$CWD dir. It's effectively a third (if optional) input.

I've pushed an update to package.json that will run ffprefs in the top-level directory, instead of cd'ing down into tools/ffprefs/.

@dosubot dosubot Bot removed the lgtm This PR has been approved by a maintainer label May 5, 2026
package.json: Use the $CARGO environment variable if set, and pass
arguments to ffprefs using the new convention (see below)

update_service_dumps.py: Allow specifying DUMPS_FOLDER and
ENGINE_DUMPS_FOLDER on the command line

ffprefs/src/main.rs: Allow specifying the prefs and engine dirs on
the command line instead of hard-coding their locations relative to
a common root dir
Comment thread tools/ffprefs/src/main.rs
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 5, 2026
@mr-cheffy mr-cheffy requested a review from Copilot May 5, 2026 12:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/ffprefs/src/main.rs
Comment thread scripts/update_service_dumps.py
@mr-cheffy mr-cheffy merged commit b93c205 into zen-browser:dev May 6, 2026
5 checks passed
@iskunk
Copy link
Copy Markdown
Contributor Author

iskunk commented May 6, 2026

Thanks! I put in a companion PR for Surfer, along similar lines.

@mr-cheffy
Copy link
Copy Markdown
Member

fails to run npm run import on windows. #13605

@iskunk
Copy link
Copy Markdown
Contributor Author

iskunk commented May 7, 2026

Hm, I wasn't aware this needed to run on Windows. The ${CARGO:-cargo} bit is likely at fault.

I see there are some solutions to running OS-specific scripts in a package.json file, so I'll review those and put together a reland.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants