Page MenuHomeFreeBSD
Differential D47008

env: Add an option to change the directory.
ClosedPublic
Actions

Authored by des on Oct 8 2024, 10:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 8:42 AM
Unknown Object (File)
Fri, Nov 22, 11:52 PM
Unknown Object (File)
Fri, Nov 22, 11:08 PM
Unknown Object (File)
Thu, Nov 21, 2:29 AM
Unknown Object (File)
Sat, Nov 9, 9:22 AM
Unknown Object (File)
Thu, Nov 7, 5:40 AM
Unknown Object (File)
Thu, Nov 7, 4:44 AM
Unknown Object (File)
Thu, Nov 7, 4:41 AM
View All 17 Files
Subscribers
0mp
bcr
imp

Details

Reviewers
0mp
bcr
Group Reviewers
Klara
Commits
rG6f6166e49c78: env: Add an option to change the directory.
Summary

This mirrors the equivalent option in GNU coreutils env, but does not
add support for long options.

Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 59848
Build 56733: arc lint + arc unit

Event Timeline

des created this revision.Oct 8 2024, 10:07 AM
Herald added a subscriber: imp. · View Herald TranscriptOct 8 2024, 10:07 AM
des requested review of this revision.Oct 8 2024, 10:07 AM
Harbormaster completed remote builds in B59831: Diff 144467.Oct 8 2024, 10:07 AM
des added a child revision: D47009: env: Check the status of stdout..Oct 8 2024, 10:07 AM
0mp added a subscriber: 0mp.Oct 8 2024, 10:58 AM

We'll need to update the manual page as well. I could do that :)

usr.bin/env/env.c
72–73

Any reason to keep it on one line?

96

What happened here?

194

Could we have it a bit more descriptive like "failed to change directory to %s" or something along those lines? Or does it attempt to follow what gnu env does?

usr.bin/env/tests/env_test.sh
125

Could you also add a negative test?

0mp added a reviewer: 0mp.Oct 8 2024, 11:06 AM
des updated this revision to Diff 144471.Oct 8 2024, 11:06 AM

incorporate review feedback

Harbormaster completed remote builds in B59834: Diff 144471.Oct 8 2024, 11:06 AM
des marked 3 inline comments as done.Oct 8 2024, 11:06 AM
0mp accepted this revision.Oct 8 2024, 11:09 AM

Thanks!

The manual page update and release notes may go in another revision.

This revision is now accepted and ready to land.Oct 8 2024, 11:09 AM
des added a comment.Oct 8 2024, 11:40 AM

Doh! The manual page had completely slipped my mind...

des edited the summary of this revision. (Show Details)Oct 8 2024, 11:49 AM
des added a reviewer: manpages.
des updated this revision to Diff 144472.Oct 8 2024, 11:50 AM
des edited the summary of this revision. (Show Details)
des removed a reviewer: manpages.

Update the manual page + error if -C or -P but no utility

This revision now requires review to proceed.Oct 8 2024, 11:50 AM
Harbormaster completed remote builds in B59835: Diff 144472.Oct 8 2024, 11:50 AM
bcr accepted this revision.Oct 8 2024, 12:25 PM
bcr added a subscriber: bcr.

OK for the man page change.

This revision is now accepted and ready to land.Oct 8 2024, 12:25 PM
0mp requested changes to this revision.Oct 8 2024, 1:18 PM

2 minor requests. Otherwise, the manual change looks OK.

usr.bin/env/env.1
516

-C should join this list. A note saying that it attempts to match GNU's behavior can be considered.

522

Would be nice to note what's the earliest release to feature -C.

This revision now requires changes to proceed.Oct 8 2024, 1:18 PM
des updated this revision to Diff 144492.Oct 8 2024, 1:48 PM

man page tweaks

Harbormaster completed remote builds in B59848: Diff 144492.Oct 8 2024, 1:48 PM
des marked 2 inline comments as done.Oct 8 2024, 1:48 PM
des marked an inline comment as done.Oct 8 2024, 1:52 PM
des added inline comments.
usr.bin/env/env.c
96

The strdup() is unnecessary and unchecked. I could either remove it or add a NULL check. Removing it was easier.

des marked an inline comment as done.Oct 8 2024, 1:52 PM
0mp accepted this revision.Oct 8 2024, 2:01 PM

Thank you!

This revision is now accepted and ready to land.Oct 8 2024, 2:01 PM
Closed by commit rG6f6166e49c78: env: Add an option to change the directory. (authored by des). · Explain WhyOct 8 2024, 5:02 PM
This revision was automatically updated to reflect the committed changes.
des added a commit: rG6f6166e49c78: env: Add an option to change the directory..
des mentioned this in rG08e8554c4a39: env: Add a handful of test cases..Oct 15 2024, 6:30 AM

Revision Contents
Changeset List

PathSize
usr.bin/
env/
15 lines
22 lines
tests/
25 lines

Diff 144492

View Options

usr.bin/env/env.1

Loading...
View Options

usr.bin/env/env.c

Loading...
View Options

usr.bin/env/tests/env_test.sh

Loading...